ponylang / ponyc

Pony is an open-source, actor-model, capabilities-secure, high performance programming language
http://www.ponylang.io
BSD 2-Clause "Simplified" License
5.69k stars 410 forks source link

Confusing error message #3610

Open SeanTAllen opened 4 years ago

SeanTAllen commented 4 years ago
primitive Foo
    fun prev(n : Bar) : String =>
        n.n2.string() + ", " + n.n1.string()

class Bar
    let n1: U8
    let n2: U8

    new create() => (n1, n2) = (1, 2)
    fun reverse() : String => Foo.prev(this)

actor Main
    new create(env: Env) =>
        let n = Bar
        env.out.print(n.reverse())

The above results in the error message

Error:
main.pony:10:40: argument not a subtype of parameter
    fun reverse() : String => Foo.prev(this)
                                       ^
    Info:
    main.pony:10:40: argument type is this->Bar ref
        fun reverse() : String => Foo.prev(this)
                                           ^
    main.pony:2:14: parameter type is Bar ref
        fun prev(n : Bar) : String =>
                 ^
    main.pony:10:40: Bar box is not a subtype of Bar ref: box is not a subcap of ref
        fun reverse() : String => Foo.prev(this)

which isn't at all helpful. to be helpful, I think it should say

main.pony:10:40: argument type is this->Bar box

I'm assuming this is a bug. It looks like a bug to me.

SeanTAllen commented 4 years ago

Here's the relevant section of code that generates the error:

https://github.com/ponylang/ponyc/blob/master/src/libponyc/expr/call.c#L243

I think the error is in here:

https://github.com/ponylang/ponyc/blob/master/src/libponyc/ast/ast.c#L1750

But I don't know this part of the codebase very well. I've never consistently done much with the AST so I'm kind of weak there.

@mfelsche @jemc comments?

stefandd commented 4 years ago

I have the feeling this is related to https://github.com/ponylang/ponyc/issues/3568#issue-623794991

jasoncarr0 commented 4 years ago

Hmm, while I agree that this error message is not perfect, but it is definitely correct (i.e. it is this->Bar ref, not box). This box method may result in something which is mutable, since it's polymorphic, it's just that in this case it's not. So the "box" part is handled by the viewpoint adaptation this->..., when this is instantiated to box.

What we may want to do is expand out the viewpoint adaptation with error messages, e.g.:

... has type this->Bar ref", but must be Bar ref. But if this has capability box, then this->Bar ref will be Bar box If this has capability val, then this->Bar ref will be Bar val

Or perhaps something like describing box->Bar ref is Bar box

jemc commented 4 years ago

Yeah, @jasoncarr0's explanation is correct - the error message is fully correct - it's just not obvious to everyone who reads it that this->ref implies a possibility of ref, box or val depending on the cap of this in a fun box.

SeanTAllen commented 4 years ago

It was agreed during sync that while the error message is technically correct, it isn't a helpful error message. Changing this is a rather deep change that involves a lot of context and knowledge of the existing compiler implementation and how and when various parts of type reification are done.

At the moment, this is a bad error message that we are mostly stuck with unless someone wants to undertake a reasonably large amount of work.

jemc commented 4 years ago

I wonder if we could get an easy win by making the compiler print this{ref, val, box}->Bar ref instead of this->Bar ref as the type - which would at least point out that the cap of this can vary and the code must account for all three capabilities.

jemc commented 4 years ago

Another option would be to add to the typechecking logic in which this->Bar ref failed to be a subtype of Bar ref, some logic that detected the this-> on the prospective subtype, and tried stripping the this-> and trying the typecheck again - if the former fails and the latter succeeds, we could print a message that says "this would be possible without the this-> viewpoint adaptation - did you mean to originate the value from a fun ref instead?". Or possibly a better wording on that same theme.