Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slightly confusing error message related to unboxing when trying to call a non-function #737

Open
jiribenes opened this issue Dec 11, 2024 · 2 comments

Comments

@jiribenes
Copy link
Contributor

jiribenes commented Dec 11, 2024

Note

This issue is probably low priority, I just wanted to write it down:

The following code tries to use a Scala-ism: using function call to call some .apply method in the style of .get:

def getFirst(words: List[String]): String = words(0)

returns the following, slightly confusing, error:

Unbox requires a boxed type, but got List[String].

It is, of course, confusing, because the user didn't write neither a box/unbox, nor a first-class function type, which causes a lot of confusion. An ideal message (given the annotations) here for a language without boxing could be something like:

Trying to call a value of type `List[String]` as a function with 1 argument of type `Int`.

(we can bikeshed this later, it also depends on the amount of information we have at that point, of course!)

Other systems:

### Elm:
The `words` value is not a function, but it was given 1 argument.

### Haskell:
- Couldn't match expected type: t0 -> String with actual type: [String]
- The function ‘words’ is applied to one value argument, but its type ‘[String]’ has none

### HMloc:
[ERROR] Type `string list` does not match `int -> _`

◉ (string list) is here
▲ - l.2 let getFirst (words: list[string]) = words 0
│ 
│  - l.2 let getFirst (words: list[string]) = words 0
│ 
◉ ('a) is assumed here
│  - l.2 let getFirst (words: list[string]) = words 0
▼ 
◉ (int -> _) is here
   - l.2 let getFirst (words: list[string]) = words 0
@jiribenes
Copy link
Contributor Author

I think the message here could be slightly more customised for situation where the unbox parent is a function call & the unbox is synthesised: 👀

case _ =>
Context.annotationOption(Annotations.UnboxParentDef, u) match {
case Some(source.DefDef(id, annot, block)) =>
// Since this `unbox` was synthesized by the compiler from `def foo = E`,
// it's possible that the user simply doesn't know that they should have used the `val` keyword to specify a value
// instead of using `def`; see [issue #130](https://github.com/effekt-lang/effekt/issues/130) for more details
Context.abort(pretty"Expected the right-hand side of a `def` binding to be a block, but got a value of type $vtpe.\nMaybe try `val` if you're defining a value.")
case _ =>
Context.abort(pretty"Unbox requires a boxed type, but got $vtpe.")
}

@jiribenes
Copy link
Contributor Author

jiribenes commented Dec 11, 2024

Here's a quick patch with a hack that allows a slightly specialised error message [error] Trying to call a value `words` of type List[String] as if it was a function. for this case:

diff --git i/effekt/shared/src/main/scala/effekt/Typer.scala w/effekt/shared/src/main/scala/effekt/Typer.scala
index 48849181..45bf4bd0 100644
--- i/effekt/shared/src/main/scala/effekt/Typer.scala
+++ w/effekt/shared/src/main/scala/effekt/Typer.scala
@@ -518,7 +518,9 @@ object Typer extends Phase[NameResolved, Typechecked] {
                 // it's possible that the user simply doesn't know that they should have used the `val` keyword to specify a value
                 // instead of using `def`; see [issue #130](https://github.com/effekt-lang/effekt/issues/130) for more details
                 Context.abort(pretty"Expected the right-hand side of a `def` binding to be a block, but got a value of type $vtpe.\nMaybe try `val` if you're defining a value.")
+              case Some(source.IdTarget(id)) =>
+                Context.abort(pretty"Trying to call a value `$id` of type $vtpe as if it was a function.")
               case _ =>
                 Context.abort(pretty"Unbox requires a boxed type, but got $vtpe.")
             }
         }
diff --git i/effekt/shared/src/main/scala/effekt/context/Annotations.scala w/effekt/shared/src/main/scala/effekt/context/Annotations.scala
index bfa619d3..ffff6c71 100644
--- i/effekt/shared/src/main/scala/effekt/context/Annotations.scala
+++ w/effekt/shared/src/main/scala/effekt/context/Annotations.scala
@@ -298,7 +298,7 @@ object Annotations {
    * Introduced by the pretyper.
    * Used by typer in order to display a more precise error message.
    */
-  val UnboxParentDef = Annotation[source.Unbox, source.Def](
+  val UnboxParentDef = Annotation[source.Unbox, source.Def | source.IdTarget](
     "UnboxParentDef",
     "the parent definition of an Unbox if it was synthesized"
   )
diff --git i/effekt/shared/src/main/scala/effekt/typer/BoxUnboxInference.scala w/effekt/shared/src/main/scala/effekt/typer/BoxUnboxInference.scala
index 7b0a47da..69e8c25b 100644
--- i/effekt/shared/src/main/scala/effekt/typer/BoxUnboxInference.scala
+++ w/effekt/shared/src/main/scala/effekt/typer/BoxUnboxInference.scala
@@ -123,7 +123,9 @@ object BoxUnboxInference extends Phase[NameResolved, NameResolved] {
     case source.ExprTarget(receiver) => source.ExprTarget(rewriteAsBlock(receiver))
     case t: source.IdTarget => t.definition match {
       case sym: (ValueSymbol | symbols.RefBinder) =>
-        source.ExprTarget(source.Unbox(source.Var(t.id).inheritPosition(t)).inheritPosition(t)).inheritPosition(t)
+        val u: source.Term.Unbox = source.Unbox(source.Var(t.id).inheritPosition(t))
+        C.annotate(Annotations.UnboxParentDef, u, t)
+        source.ExprTarget(u.inheritPosition(t)).inheritPosition(t)
       case sym: BlockSymbol =>
         t
     }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant