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

Unary + precedence #1016

Open
kddnewton opened this issue Jun 4, 2024 · 5 comments
Open

Unary + precedence #1016

kddnewton opened this issue Jun 4, 2024 · 5 comments
Labels

Comments

@kddnewton
Copy link

There are two issues with unary + precedence that I can find.

The first is that the parser is creating a send node, when there isn't really going to be a method call. This is, for example:

class Integer; def +@; self * 10; end; end

+5 ** 2 # => 25
+ 5 ** 2 # => 2500

As you can see, the space affects the result. If there is no space, then no method call is generated. However, in both cases you get the same AST:

(send
  (send
    (int 5) :**
    (int 2)) :+@)

The other issue is that the precedence in this AST is incorrect in both cases. This is implying that the expression should be parsed as:

+(5 ** 2)

but in reality, in both cases it should be parsed as:

(+5) ** 2 # => 25
(+ 5) ** 2 # => 2500
@marcandre
Copy link
Contributor

marcandre commented Jun 4, 2024

You must note that this is particular to integer constants.
+5 is interpreted by Ruby as a literal corresponding to5, but foo = 5; +foo will behave how you expected it to.

OTOH, there does indeed seem to be a bug in the result from parser, in the case of +5 I think there shouldn't be a send :@+ at all.

Note that this is only the case with integer literals:

$ ruby-parse -e "foo = 5; +foo ** 2"
(begin
  (lvasgn :foo
    (int 5))
  (send
    (send
      (lvar :foo) :+@) :**
    (int 2)))

@kddnewton
Copy link
Author

I believe this will be a problem with any number constants, not just integers.

@marcandre
Copy link
Contributor

Sorry, I did mean number, you are correct.

@iliabylich
Copy link
Collaborator

Thanks for reporting. Does this bug (or #1017) impact Prism's test suite? If not I'll fix it "later", nobody else seems to need it.

@iliabylich iliabylich added the bug label Jun 4, 2024
@kddnewton
Copy link
Author

Thanks @iliabylich no rush at all, I'm just excluding those tests from the test suite that compares the AST translation. So it's no rush.

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

No branches or pull requests

3 participants