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

List of overloads shown on compiler error doesn't replace type parameters #12170

Closed
asterite opened this issue Jun 29, 2022 · 10 comments
Closed

Comments

@asterite
Copy link
Member

Consider this code:

a = [1, 2, 3]
a.push 'a'

The error we get is this:

In foo.cr:2:3

 2 | a.push 'a'
       ^---
Error: no overload matches 'Array(Int32)#push' with type Char

Overloads are:
 - Array(T)#push(value : T)
 - Array(T)#push(*values : T)

Note that the method is defined as push(value : T) and it shows up like that, but here the compiler knows that T is Int32 so it could tell the user that, instead of having them do the replacement.

Ideally, the error should look like this:

In foo.cr:2:3

 2 | a.push 'a'
       ^---
Error: no overload matches 'Array(Int32)#push' with type Char

Overloads are:
 - Array(Int32)#push(value : Int32)
 - Array(Int32)#push(*values : Int32)
@beta-ziliani
Copy link
Member

Two little concerns:

  1. With big methods you might lose which parameter is type-dependent if there are others that match. Consider having an overload of Array(T)#push(value : T, times : Int32, with T = Int32.
  2. Having a complex union type as the argument of the parametric type might obfuscate the search.

@asterite
Copy link
Member Author

That's a good point.

Maybe I can send a PR with everything I have in mind to improve these errors, instead of suggesting or tackling one thing at a time 😊

@asterite asterite reopened this Jun 29, 2022
@asterite
Copy link
Member Author

Actually this is unrelated to the other error message improvement...

@asterite
Copy link
Member Author

I agree with those concerns, but if we go with #11106 then it won't matter because you'll have a short summary explaining why each overload didn't match.

@straight-shoota
Copy link
Member

Perhaps we could just enhance the current output to include the value of type parameters. For example like this:

1) Array(T: Int32)#push(value : T)
2) Array(T)#push(value : T) with T: Int32

This addresses the concerns mentioned in #12170 (comment) (which I share).

I think I like 2) better despite being a bit more verbose, but it's probably not a big difference.

@asterite
Copy link
Member Author

I think in both cases you still have to do the replacement in your head. I just think that, even if it's more verbose, it's going to be clearer if type parameters are replaces. Maybe we can also check how other languages do this.

@asterite
Copy link
Member Author

Here's Java.

This is the code:

import java.util.*;

public class MyClass {
    public static void main(String args[]) {
      ArrayList<String> a = new ArrayList<String>();
      a.add(1, 2, 3, 4);
    }
}

I get this error:

/MyClass.java:6: error: no suitable method found for add(int,int,int,int)
      a.add(1, 2, 3, 4);
       ^
    method ArrayList.add(String,Object[],int) is not applicable
      (actual and formal argument lists differ in length)
    method ArrayList.add(String) is not applicable
      (actual and formal argument lists differ in length)
    method ArrayList.add(int,String) is not applicable
      (actual and formal argument lists differ in length)
1 error

Note that "T" has been replaced.

@straight-shoota
Copy link
Member

For what it's worth: In Java you don't have type unions, so the output is typically more concise. But you also often have longer namespaces which might balance that to some degree.

@asterite
Copy link
Member Author

Yeah, I just tried it with a union type and it doesn't look very good.

@zw963
Copy link
Contributor

zw963 commented Jul 2, 2022

I really think original compile error is better.

In foo.cr:2:3

 2 | a.push 'a'
       ^---
Error: no overload matches 'Array(Int32)#push' with type Char  # <= 1. here told user, it accept Int, but you push a Char.

Overloads are:
 - Array(T)#push(value : T)    # <= 2. here show the REAL overloads definition is here, it good, more is redundant.
 - Array(T)#push(*values : T)

From my point of view, the issue is, we should improve 1's readability, as described in #11106 , it better.
but, even, if we can output 1's content as 2, it improvement a lot.

following is a really example come from avaram, a very small user table.

After chang to wrong changes, i get error message like following: (please check screenshot)

image

we know some of type is wrong, but, we have to find in below overload error msg, It Crazy.

Even, if we can format the type art like this, it better.

 - Array(Int32)#push(
      - value : Int32
   )
 - Array(Int32)#push(
      - *values : Int32
   )

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

Successfully merging a pull request may close this issue.

4 participants