Improve error message for missing field name in tuple

[plajjan]

I think the parsing error for this could be improved.

The last field in my tuple is missing its name:

actor main(env):
    abc = (a="foo", "bar")

kll@Boxy:~/terastream/acton$ actonc examples/foo.act
Building file examples/foo.act

ERROR: Error when compiling foo module: Syntax error
examples/foo.act:2:7:
  |
2 |     abc = (a="foo", "bar")
  |         ^^
unexpected "= "
expecting ':', ';', call arguments, slice/index expression, comma, end of line, if clause, or operator

and it's pointing to what?

[plajjan]

@sydow I think this counts as a parser error, like you were looking for in #732, right? :)

[sydow]

Yes, this is certainly a good example of a terrible error message. I will be glad to try to fix it as soon as I know what syntax should be allowed. For example, this is accepted by the compiler:

actor main(env):
abc = ("foo", b="bar")

and abc gets type (str, b : str), following the idea (for function parameters) that positional elements precede keyword ones. What are the rules for tuples with (some) named fields? Maybe this was discussed at some point, but I certainly don't remember. I am sure @nordlander does.

[plajjan]

@sydow I have not been part of any such discussion - I don't think we've discussed the details of it in particular. As far as I know, the design that we've implemented is to allow tuples with either named fields or no names. Thus, mixing by providing names for some fields but not others is invalid and this seems to be enforced now (although perhaps more by happenstance!?). I don't really have an opinion on whether it makes sense to allow mixing or not. Since names are just an alias that get replaced by the index I guess it's technically possible to mix. What are the drawbacks to mixing?

In the current situation with named / unnamed mix being forbidden, the best error message is the one that guides the user to do the right thing and fix their code.

I think this looks great:

ERROR: Error when compiling foo module: Syntax error
  ╭─[ examples/foo.act:2:7 ]
  │
2 │     abc = (a=1, b=2, c=3, 4)
  .            ─┬─  ─┬─  ─┬─  ┬
  .             │    │    │   ╰──── This field does not have a name
  .             │    │    │
  .             ╰────┴────┴──────── These fields have a name
  ╰──
Error: Tuple contains a mix of named and unnamed fields.
Hint: All fields in a tuple must be named or unnamed. Either add names to all fields or remove names to make the tuple unnamed.

granted, it does quite a bit more stuff than our current error messages but I think it's the direction we should move in. This whole Error / Hint thing is one that Postgresql has implemented and it's been sooooo useful so I'd really like to have that. The ascii style drawings of things is inspired by both Haskell's error messages but also Ariadne (see #490).

[nordlander]

Indeed, the idea was that tuples and function arguments would support exactly the same syntax. Unfortunately, when we built the first Acton parser we attempted to follow Python's official but messy grammar to the letter, and while we've dropped that goal in later revisions I think we're still struggling with traces of Python grammar leftovers in the tuple and argument parser definitions. Anyway, what I think we want to support is the following. A tuple -- or a function argument list -- consists of (from left to right): - Zero or more positional arguments - An optional star argument - Zero or more keyword arguments - An optional double-star argument So the expression (a="foo", "bar") is a syntactical error, while ("foo", b="bar") is correct. Well, this is the idea, at least. In addition, the constraint solver will match tuple types as well as function arguments by applying the following adjustments: - Surplus given positional arguments will count as keyword arguments in their expected order, unless a star argument is also expected - A deficit of given positional arguments will be accepted provided that the missing argument types are all optional - Same for missing keyword arguments (It is the implementation of these three rules, including the corresponding term rewrites, that I'm working on right now.)

…

-- Johan

On 2 Oct 2023, at 08:31, sydow ***@***.***> wrote: Yes, this is certainly a good example of a terrible error message. I will be glad to try to fix it as soon as I know what syntax should be allowed. For example, this is accepted by the compiler: actor main(env): abc = ("foo", b="bar") and abc gets type (str, b : str), following the idea (for function parameters) that positional elements precede keyword ones. What are the rules for tuples with (some) named fields? Maybe this was discussed at some point, but I certainly don't remember. I am sure @nordlander does. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[sydow]

Hej! Detta stämmer med min vaga minnesbild och är nog vad vi implementerat — men med ett bedrövligt felmeddelande i Kristians exempel. Ska försöka fixa det även om det inte blir så bra som Kristians förslag. För närvarande blir det litet mindre dåligt om man gör samma fel i en funktionsapplikation: def f(x,y): return x+y actor env(main): print(f(x=3,y)) ger felmeddelandet ERROR: Error when compiling tuple module: Syntax error newexamples/tuple.act:5:17: | 5 | print(f(x=3,y)) | ^ unexpected 'y' expecting "**" or a closing parenthesis Inte så dumt; efter ett kwd-argument kan endast följa ett **-dito. Björn

…

On 2 Oct 2023, at 10:19, Johan Nordlander ***@***.***> wrote: Indeed, the idea was that tuples and function arguments would support exactly the same syntax. Unfortunately, when we built the first Acton parser we attempted to follow Python's official but messy grammar to the letter, and while we've dropped that goal in later revisions I think we're still struggling with traces of Python grammar leftovers in the tuple and argument parser definitions. Anyway, what I think we want to support is the following. A tuple -- or a function argument list -- consists of (from left to right): - Zero or more positional arguments - An optional star argument - Zero or more keyword arguments - An optional double-star argument So the expression (a="foo", "bar") is a syntactical error, while ("foo", b="bar") is correct. Well, this is the idea, at least. In addition, the constraint solver will match tuple types as well as function arguments by applying the following adjustments: - Surplus given positional arguments will count as keyword arguments in their expected order, unless a star argument is also expected - A deficit of given positional arguments will be accepted provided that the missing argument types are all optional - Same for missing keyword arguments (It is the implementation of these three rules, including the corresponding term rewrites, that I'm working on right now.) -- Johan > On 2 Oct 2023, at 08:31, sydow ***@***.***> wrote: > > > Yes, this is certainly a good example of a terrible error message. I will be glad to try to fix it as soon as I know what syntax should be allowed. For example, this is accepted by the compiler: > actor main(env): > abc = ("foo", b="bar") > and abc gets type (str, b : str), following the idea (for function parameters) that positional elements precede keyword ones. What are the rules for tuples with (some) named fields? Maybe this was discussed at some point, but I certainly don't remember. I am sure @nordlander does. > — > Reply to this email directly, view it on GitHub, or unsubscribe. > You are receiving this because you were mentioned.Message ID: ***@***.***> — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[nordlander]

Ja, för anrop blir felmeddelandet mycket bättre! Och man borde kunna parsa tupler på precis samma sätt, tycker jag. Det som möjligen krånglar till det är att grammatiken tillåter att man utelämnar parenteserna runt en tupel på vissa ställen. Men det borde gå att begränsa om det ställer till problem. Det enda ställe där jag spontant tycker att tupler utan parentes har någon som helst poäng är vid retur av multipla funktionsresultat. Och även där är vinsten ganska blygsam.

[sydow]

fixed by #1597