chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.8k stars 421 forks source link

Simplify/specialize error message for calling a method on a nilable class variable #25153

Open bradcray opened 5 months ago

bradcray commented 5 months ago

Currently, when calling a method on a nilable class, the error message we get is quite long and quite general. E.g., for this test [ATO]:

class C {
  proc foo() { writeln("In C.foo()"); }
}

var myC: C?;
myC = new C();
myC.foo();

we get this error:

code.chpl:7: error: unresolved call 'owned C?.foo()'
code.chpl:2: note: this candidate did not match: C.foo()
code.chpl:7: note: because method call receiver with type 'owned C?'
code.chpl:2: note: is passed to formal 'this: borrowed C'
code.chpl:7: note: try to apply the postfix ! operator to method call receiver

Each time I get one of these I go into a minor panic because the error message is so long and general it makes me think I did something very wrong—particularly when the method is more complex, taking more arguments that could conceivably be mismatched, etc.

It seems to me that it would be nice if we just said something like:

code.chpl:7: error: in Chapel, methods cannot be called on nilable classes
code.chpl:7: note: consider making the class non-nil by applying the postfix '!' operator, casting away its nilability, or …?

Or is there a reason that this isn't appropriate that I'm forgetting about?

mppf commented 5 months ago

It is actually a "unresolved call" / "candidate did not match" error; in that it's possible to add an overload to accept a nilable version of the class. I agree that isn't normally the right way to fix it; it's more just that if we completely take away the words about resolving / candidates, it might not be clear that this is an error coming from a failure to resolve a method call.

Put another way, the reason the message is long today is that it's using the "unresolved call" format, which makes sense in the implementation (and whether it makes sense to someone who runs into this is IMO the main thing to think about here).

bradcray commented 5 months ago

in that it's possible to add an overload to accept a nilable version of the class.

Ah, that's a good point, and one that I was missing when writing this. To make sure it's clear to others, you could define foo to be a secondary method like:

proc (C?).foo() { writeln("In C.foo()"); }

in which case the program in the OP works.

I still think it'd be more ergonomic if the error message logic were to: