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

Several named capture warnings in date/format #3865

Closed
headius opened this Issue May 9, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@headius
Member

headius commented May 9, 2016

Environment

JRuby 9k versions through 9.1.

Expected Behavior

Ideally none of the standard library sources should produce warnings in verbose mode.

Actual Behavior

Our date/format.rb, created by @eregon, produces the following errors when loaded in verbose mode:

/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:960: warning: named capture conflicts a local variable - year
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:960: warning: named capture conflicts a local variable - mon
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:960: warning: named capture conflicts a local variable - mday
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:960: warning: named capture conflicts a local variable - yday
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:960: warning: named capture conflicts a local variable - cwyear
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:960: warning: named capture conflicts a local variable - cweek
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:960: warning: named capture conflicts a local variable - cwday
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:960: warning: named capture conflicts a local variable - hour
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:960: warning: named capture conflicts a local variable - min
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:960: warning: named capture conflicts a local variable - sec
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:960: warning: named capture conflicts a local variable - sec_fraction
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:960: warning: named capture conflicts a local variable - zone
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:1005: warning: named capture conflicts a local variable - hour
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:1005: warning: named capture conflicts a local variable - min
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:1005: warning: named capture conflicts a local variable - sec
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:1005: warning: named capture conflicts a local variable - sec_fraction
/Users/headius/projects/jruby/lib/ruby/stdlib/date/format.rb:1005: warning: named capture conflicts a local variable - zone

These errors are Ruby's attempt to warn us that we're using the same local variable name as a named capture. I do not believe this warning is useful, and I have filed https://bugs.ruby-lang.org/issues/12359 to have it removed from Ruby. If that bug is not accepted, we'll need to change this code to eliminate the warnings.

@eregon

This comment has been minimized.

Show comment
Hide comment
@eregon

eregon May 9, 2016

Member

One way to avoid it would be to put each branch in a different scope, either blocks or methods. But that probably has some overhead.
Do we want to keep the exact same warnings as MRI, even on stdlib code? Otherwise that warning could be disabled for that known file (yes it's a hack, but are the other workarounds better?).
Thanks for filing the MRI issue!

Member

eregon commented May 9, 2016

One way to avoid it would be to put each branch in a different scope, either blocks or methods. But that probably has some overhead.
Do we want to keep the exact same warnings as MRI, even on stdlib code? Otherwise that warning could be disabled for that known file (yes it's a hack, but are the other workarounds better?).
Thanks for filing the MRI issue!

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius May 9, 2016

Member

Yeah, that's another way to fix it. I'd rather we just remove the warning altogether, though, since I think the only options for users to remove it are all gross. We'll see what ruby-core thinks.

Member

headius commented May 9, 2016

Yeah, that's another way to fix it. I'd rather we just remove the warning altogether, though, since I think the only options for users to remove it are all gross. We'll see what ruby-core thinks.

@headius headius closed this in 3ebc198 May 10, 2016

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius May 10, 2016

Member

For now I've fixed our issues by modifying the warning to only fire when a named capture has already been declared as a non-named-capture. I'll follow up with ruby-core on the remaining issue.

Member

headius commented May 10, 2016

For now I've fixed our issues by modifying the warning to only fire when a named capture has already been declared as a non-named-capture. I'll follow up with ruby-core on the remaining issue.

@headius headius added this to the JRuby 9.1.1.0 milestone May 10, 2016

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