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

+ Added emit_forward_arg compatibility flag. #710

Merged

Conversation

iliabylich
Copy link
Collaborator

The reason is that Ruby 2.8 supports the following code:
def m(a, b, ...); end

If set to false (the default):

  1. def m(...) end is emitted as

    s(:def, :m, s(:forward_args), nil)
    
  2. def m(a, b, ...) end is emitted as

    s(:def, :m,
      s(:args, s(:arg, :a), s(:arg, :b), s(:forward_arg)))
    

If set to true it uses a single format:

  1. def m(...) end is emitted as

    s(:def, :m, s(:args, s(:forward_arg)))
    
  2. def m(a, b, ...) end is emitted as

    s(:def, :m, s(:args, s(:arg, :a), s(:arg, :b), s(:forward_arg)))
    

This is necessary to backport ruby/ruby@f8b4340

I'll keep it open for a few days, feel free to leave comments/suggestions.

The reason is that Ruby 2.8 supports the following code:
  def m(a, b, ...); end

If set to false (the default):
  1. `def m(...) end` is emitted as
     s(:def, :m, s(:forward_args), nil)
  2. `def m(a, b, ...) end` is emitted as
     s(:def, :m,
       s(:args, s(:arg, :a), s(:arg, :b), s(:forward_arg)))

If set to true it uses a single format:
  1. `def m(...) end` is emitted as
     s(:def, :m, s(:args, s(:forward_arg)))
  2. `def m(a, b, ...) end` is emitted as
     s(:def, :m, s(:args, s(:arg, :a), s(:arg, :b), s(:forward_arg)))
@marcandre
Copy link
Contributor

Just curious, when is s(:def, :m, s(:forward_args), nil) desirable?

@iliabylich
Copy link
Collaborator Author

Never. It was a wrong decision initially, but some code can rely on it, so we can't break it easily. Ideally users should enable the flag and use the new format.

@iliabylich iliabylich merged commit c776771 into whitequark:master Jun 15, 2020
@iliabylich iliabylich deleted the add-comp-flag-for-forward-args branch June 15, 2020 17:22
@marcandre
Copy link
Contributor

Is a release planned for emit_forward_arg? I'd like to enable it by default in RuboCop

@iliabylich
Copy link
Collaborator Author

@marcandre 2.7.1.4 has been released - https://rubygems.org/gems/parser/versions/2.7.1.4

@marcandre
Copy link
Contributor

❤️ Thank you @iliabylich ❤️

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

Successfully merging this pull request may close these issues.

None yet

3 participants