confusing error message for bad method application

[shriram]

Program:

data Foo:
  | m(x)
    with:
    method get(y): y * 2 end
end

Run it with

> v = m(3)
> v.get(10)

The error message says Notice that the green highlight that claims to show "2 arguments" shows only one!

(I also find awkward the wording "was a function defined accepting 1 argument" rather than "was a function defined to accept 1 argument".)

Observe that if I have a regular function instead, the highlighting works correctly: But the hidden argument makes this error highlighting very confusing.

[blerner]

I can obtain the srcloc for the v in your example. Not sure it helps clarify things, but it's a start. Another thing that's unclear in this message is it claims that v.get is a function application, but it's clearly method application. This can only mostly be detected and fixed: in the example below,

obj = {foobar: lam(x): x end}
obj.foobar(3)

We can't know from the error that the obj.foobar is a lambda, not a method. I think it's worth changing the wording of the message to say "method application expression" in this instance anyway, because "function application expression" is going to be misleading for the many method applications we're going to have with the new table API.

(Also -- does anyone recall why we're saying "function application expression", and not just "application expression"? If we can shorten it, then we can dodge this problem entirely...)

[schanzer]

I think I prefer "application expression". It's got a lower cognitive load and is certainly easier to say. @shriram , can you think of a counterexample where it would useful to give the user feedback about whether what's being applied is either a method or a function?

[jswrenn]

Also -- does anyone recall why we're saying "function application expression", and not just "application expression"? If we can shorten it, then we can dodge this problem entirely...

I wrote "function application" rather than just "application" because it reinforces a symmetry with "function definition" and is maybe a little more google-able. That said, the docs refer to it as an "application expression", and given @blerner's snippet, I'd far prefer "application expression" than presenting a sometimes misleading error message.

[blerner]

Then you can revert most of the commit I made above. (The commit also includes some typo fixes that you should keep.on..) The receiver parameter is going to be misleading sometimes, and shouldn't be highlighted. And we'll still need to come up with some wording to explain the off-by-one counting for arguments to methods...

[jpolitz]

Arity is checked in the body of functions and methods. It should be straightforward to have this particular error be different in method bodies (via an extra parameter to compile-fun-body that selects the right error to throw). Then we can use that information to decide how to report the message.

[blerner]

Does that hold up to currying? If we say foo = o.m, then foo is now a regular-seeming function, but it's compiled to contain an error specific to methods...

[jpolitz]

It does. The curry-created function doesn't arity-check, and just delegates to the method, which gets the full list (including self), and contains the arity check.

[blerner]

But calling foo(...) looks like a function call, but the error will still complain about methods.

[jpolitz]

Two responses:

1. My understanding is that the problem here is the lack of information about method-ness of the applied thing. Knowing method-ness lets us do a better job of describing the number of arguments, and saying "applied to one argument with receiver object { ... }", rather than "applied to two arguments" when the application in the source appears to only have one argument. The extra information is needed in either case to disambiguate the error message. I claim that the foo(...) call to a curried method ought to say something about methods in the arity error case.

2. We can always add a third error message for arity mismatches on curried method calls. It has a cost, but curried methods already introduce a cost so they're more a convenience feature than an optimized one.

[shriram]

@jswrenn and I talked about this sort of situation yesterday. We had some term similar to “application” before and got rid of it. I had told Jack to ignore function vs. method and just use the word function — my reasoning is that someone who knows what they're doing won't notice, and those who don't in some sense won't either. Remember that one of the lessons of Guillaume's "mind your language" paper is that the more we used abstract/refined terminology, the less students had any idea what the error messages meant. That's what really worries me about using “application”. Nevertheless, if we do use that, why do we need to say “application expression” instead of just “application”? But I still don't like the term…

[blerner]

Part of my distaste for "application", by its lonesome, is that the other meaning of that word is "the entire program you're trying to write", especially with the prevalence of "apps" everywhere. So having "expression" in the wording seems like a win. Having "function" vs "method" seems like a loss. And so does trying to highlight the receiver object if there is one, since it can be misleading. Granted, it's not misleading in the common case, but I still don't like it...

[shriram]

s/application/call/ ?

[blerner]

Ok, so that gives the options "call", "function call", "call expression", "function call expression", and two of those still have the function/method issue. I think "call expression" is probably the best of that bunch...

[jswrenn]

bbdc98f415c27d39ca1f838b17314d0f80c4672b makes changes to arity-mismatch and an overview of those changes is given in https://github.com/brownplt/pyret-lang/commit/bbdc98f415c27d39ca1f838b17314d0f80c4672b#commitcomment-23300156.

As it stands, the wording I am using is "application expression". I don't have any qualms about changing it to "call expression", but that change will need to be coordinated with the pyret documentation.

I am going to follow up with similar changes to constructor-arity-mismatch and special wording for method applications (implementing https://github.com/brownplt/pyret-lang/issues/1004#issuecomment-312916503).