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

Confusing error message for different types with same name #326

Open
jiribenes opened this issue Dec 1, 2023 · 9 comments
Open

Confusing error message for different types with same name #326

jiribenes opened this issue Dec 1, 2023 · 9 comments

Comments

@jiribenes
Copy link
Contributor

Trying to write a sort that works on the first element of a tuple:

import immutable/list

def comparePairsByFirst[A, B](x: Tuple2[A, B], y: Tuple2[A, B]){ compare: (A, A) => Boolean }: Boolean = {
    val (a1, b1) = x
    val (a2, b2) = y
    compare(a1, a2)
}

def sortOnFst[Int, B](lst: List[Tuple2[Int, B]]): List[Tuple2[Int, B]] = {
    lst.sortBy { (p1, p2) => 
      comparePairsByFirst(p1, p2) { 
        (a, b) => a < b 
      } 
    }
}

This is reproducible both on the website and on current master.

I got the following error message:

Cannot typecheck call.
There are multiple overloads, which all fail to check:

Possible overload: effekt.infixLt of type (Int, Int) => Boolean
  Expected Int but got Int.
  Expected Int but got Int.

Possible overload: effekt.infixLt$1 of type (Double, Double) => Boolean
  Expected Double but got Int.
  Expected Double but got Int.

Possible overload: effekt.infixLt$2 of type (String, String) => Boolean
  Expected String but got Int.
  Expected String but got Int.

Specifically, the very first part seems wrong:

Possible overload: effekt.infixLt of type (Int, Int) => Boolean
  Expected Int but got Int.
  Expected Int but got Int.

This overload looks like it should work in my case (I'm comparing two Ints), but it gets refused for some mysterious reason.

@jiribenes jiribenes added the bug Something isn't working label Dec 1, 2023
@jiribenes
Copy link
Contributor Author

Note: type annotations don't help:

def sortFst[Int, B](lst: List[Tuple2[Int, B]]): List[Tuple2[Int, B]] = {
    lst.sortBy { (p1: Tuple2[Int, B], p2: Tuple2[Int, B]) => 
      comparePairsByFirst(p1, p2) { 
        (a: Int, b: Int) => a < b 
      } 
    }
}

@jiribenes
Copy link
Contributor Author

@b-studios helpfully told me that the problem is here:

def sortFst[Int, B]

^ we're binding Int as a type variable, so the error should actually say something like:

Possible overload: effekt.infixLt of type (effekt/Int, effekt/Int) => Boolean
  Expected effekt/Int but got main/Int.
  Expected effekt/Int but got main/Int.

@jiribenes jiribenes added errormessage and removed bug Something isn't working labels Dec 1, 2023
@jiribenes jiribenes changed the title Overloading less-than operator fails unexpectedly Confusing error message for different types with same name Dec 1, 2023
@b-studios
Copy link
Collaborator

We could check whether the two types print to the same and then use the fully qualified name.

@b-studios
Copy link
Collaborator

This should be (partially) addressed by #368. @jiribenes do you think this issue here is still relevant?

@jiribenes
Copy link
Contributor Author

It's better since now there's an additional message:

Type parameter Int shadows outer definition of Int

which is already helpful.

But since we have namespaces, it would be nice to say something like:

Possible overload: effekt::infixLt of type (Int, Int) => Boolean
  Expected effekt::Int but got main::Int.
  Expected effekt::Int but got main::Int.
           ^^^^^^^^            ^^^^^^

Some logic like "If the types are named the same, print their whole qualified path". :)

@marvinborner
Copy link
Member

I've had a similar issue today while working on probabilistic.effekt in #776 after importing list for its show function. By now the reason for this error is obvious, since the file already had a type definition for List, but it still cost me a lot of time.

... (bunch of possible overloads)

Possible overload: list::println of type List[String] => Unit
  Expected List[String] but got List[String].

... (bunch of possible overloads)

Reproduction:

import list

type List[A] {
  Nil();
  Cons(head: A, tail: List[A])
}

def main() = {
  val list = Cons("foo", Cons("Bar", Nil()))
  println(list)
}

In my opinion, such code should throw some error message like "ambiguous type 'List[String]'".

@jiribenes
Copy link
Contributor Author

To reiterate my point from a year ago, I think we should print the "fully qualified" path (or the origin) if we notice we're writing a message like "Expected X but got X".
(There are smarter ways of doing this, but this is the quickest)

So something like:

  Expected effekt::List[effekt::String] but got probabilistic::List[effekt::String].

would already help in these "Expected X but got X" cases.

@marvinborner
Copy link
Member

I don't have any experience with Typer but I suppose we should add this check around here?

def matchExpected(got: ValueType, expected: ValueType)(using Context): Unit =
Context.requireSubtype(got, expected,
ErrorContext.Expected(Context.unification(got), Context.unification(expected), Context.focus))
def matchExpected(got: BlockType, expected: BlockType)(using Context): Unit =
Context.requireSubtype(got, expected,
ErrorContext.Expected(Context.unification(got), Context.unification(expected), Context.focus))

How would we attach the respective module name?

@jiribenes
Copy link
Contributor Author

I don't have any experience with Typer but I suppose we should add this check around here?

How would we attach the respective module name?

Asked on the meeting:

  1. We have toDoc for types for pretty printing, but it uses unqualified names. We should make a new pretty printer that uses qualified names (a Name can be LocalName or QualifiedName)
  2. Then here, we should separate out a case where expRendered == gotRendered, then use the qualified name:
    case Expected(got, exp, tree) =>
    val expRendered = pp"$exp"
    val gotRendered = pp"$got"
    val msg = if ((expRendered.size + gotRendered.size) < 25) {
    s"Expected $expRendered but got $gotRendered."
    } else {
    s"Expected type\n $expRendered\nbut got type\n $gotRendered"
    }
    if (tpe1 != got || tpe2 != exp)
    pp"$msg\n\nType mismatch between $tpe2 and $tpe1."
    else
    msg

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

3 participants