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

No end of expression on grouped stab #13358

Closed
mhanberg opened this issue Feb 19, 2024 · 4 comments
Closed

No end of expression on grouped stab #13358

mhanberg opened this issue Feb 19, 2024 · 4 comments

Comments

@mhanberg
Copy link
Member

Elixir and Erlang/OTP versions

Erlang/OTP 26 [erts-14.2.1] [source] [64-bit] [smp:32:32] [ds:32:32:10] [async-threads:1] [jit:ns]

Elixir 1.17.0-dev (compiled with Erlang/OTP 26)

[tools]
erlang = "26.2.1"
elixir = "ref:514615d0347cb9bb513faa44ae1e36406979e516"

Operating system

Linux

Current behavior

The following code produces the following AST

a do
  d ->
    (b -> c)
end
{
  :a,
  [
    end_of_expression: [newlines: 1, line: 4, column: 4],
    do: [line: 1, column: 3],
    end: [line: 4, column: 1],
    line: 1,
    column: 1
  ],
  [
    [
      do: [
        {:->, [newlines: 1, line: 2, column: 5],
         [
           [{:d, [line: 2, column: 3], nil}],
           [{:->, [line: 3, column: 8], [[{:b, [line: 3, column: 6], nil}], {:c, [line: 3, column: 11], nil}]}]
         ]}
      ]
    ]
  ]
}

Expected behavior

Should the grouped stab expression (b -> c) have end_of_expression data?

This is a weird case, in that its its more like a child expression and less like prongs of a do block.

But, if you were to have this code in a type spec, I'm not what I would expect to happen.

Also, I'll admit while this is valid syntax, its not valid code outside of a macro, I don't think.

@josevalim
Copy link
Member

The issue is that there is no end of line expression for b -> c. We could add around (b -> c) but, since it is a list, we have no place to hang metadata. We should debate wrapping (b -> c) using the literal encoder (instead of an immediate list).

@mhanberg
Copy link
Member Author

there is no end of line expression

Are you sure? The tokenizer produces this

{:ok, 4, 4, [],
 [
   {:do_identifier, {1, 1, ~c"a"}, :a},
   {:do, {1, 3, nil}},
   {:eol, {1, 5, 1}},
   {:identifier, {2, 3, ~c"d"}, :d},
   {:stab_op, {2, 5, nil}, :->},
   {:eol, {2, 7, 1}},
   {:"(", {3, 5, nil}},
   {:identifier, {3, 6, ~c"b"}, :b},
   {:stab_op, {3, 8, nil}, :->},
   {:identifier, {3, 11, ~c"c"}, :c},
   {:")", {3, 12, nil}},
   {:eol, {3, 13, 1}},
   {:end, {4, 1, nil}}
 ]}

There is an :eol token right after the closing paren. But now that I type this out, I assume you mean there isn't a "closing_paren_eoe" item in the parser?

Also to clarify, I am not sure what the right answer is, I just noticed the difference.

For my purposes of making Spitfire match the yecc parser, I can easily just make right stabs never have eoe metadata for now.

@josevalim
Copy link
Member

There is an :eol token right after the closing paren. But now that I type this out, I assume you mean there isn't a "closing_paren_eoe" item in the parser?

Exactly. There is an expression closing (...) but not what is inside of it. However, the AST for (...) is a list, so there is no place to attach metadata. We need to wrap its AST in a block literal as any other list to address this and have the metadata available.

@mhanberg
Copy link
Member Author

That makes sense.

Does it make sense to pass it through the literal encoder or to create a new encoder for grouped expressions?

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

No branches or pull requests

2 participants