From: merch-redmine@... Date: 2020-01-22T22:44:10+00:00 Subject: [ruby-core:96974] [Ruby master Bug#16519] pp [Hash.ruby2_keywords_hash({})] shows `[nil]` Issue #16519 has been updated by jeremyevans0 (Jeremy Evans). Eregon (Benoit Daloze) wrote: > jeremyevans0 (Jeremy Evans) wrote: > > With ruby2_keywords by default, it's used for all methods accepting args splat and not keywords, making this type of issue much more common. > > Do you have any existing real code example where that branch causes problems? Do you mean other than the example in this ticket? No. Do I expect other real world examples like this one exist? Yes. > I agree it is more likely to have a few problems similar to this one than explicit `ruby2_keywords`. > OTOH, it would also make migrating thousands of delegation calls (i.e., all delegation cases passing kwargs) so much easier. > It's a trade-off. I'd take one case like this needing `, **{})` over finding where to add 1000 explicit `ruby2_keywords` any day. It is a trade-off. I agree that in the majority of cases, `ruby2_keywords` by default does what you want. However, I don't think that's worth the cases where it doesn't, such as this one. A codebase that does not use `ruby2_keywords` would never be subject to these issues, but if you have `ruby2_keywords` by default, you can be subject to these issues even if you never use `ruby2_keywords` in any code. > > This is by design. Empty keyword splats are equivalent to passing no arguments, just as empty regular splats are equivalent to passing no arguments. I consider the Ruby <2.7 behavior a bug in this regard. When you consider that `**{}` never passed arguments (was removed by the parser), I think the intended behavior for empty keyword splats was not to pass arguments. > > Yes, I understand this point of view. But the fact is when calling a method not accepting keyword arguments, with syntactic keywords, there is inconsistency between empty keyword arguments and non-empty as shown above. > That's clearly not ideal and makes thinking about `**` complicated because it might pass a positional argument or not. > It's something we could have avoided by raising if the method does not accept keyword arguments (but that's probably too incompatible). I disagree that it is "clearly not ideal". I think it makes the most sense and is the ideal behavior. Conceptually, an empty keyword splat is no arguments, unlike an empty positional hash, which is a single empty argument. Likewise, a non-empty keyword splat is some arguments. For a method that does not accept keywords, any keyword arguments are combined into a single hash, if any keyword arguments are provided. The following two cases should be treated the same (assume `foo` does not accept keywords): ``` foo() foo(**{}) ``` In both cases, no keywords are passed to foo. > > I think the current behavior of always removing empty keyword splats (including implicit splats of empty flagged hashes) makes much more sense. > > It's also the only construct in Ruby where a Hash argument can fully disappear. When you do `foo(**{})`, you can consider the hash disappearing. So treating `foo(*[empty_flagged_hash])` the same is consistent. > I'd argue if the user passed keyword arguments (e.g., with `**h`), even empty they should be preserved and not ignored, especially when calling a method not accepting keyword arguments. > I think removing it is counter-intuitive in some cases: for example it causes these weird cases for `p(**empty_hash)` which prints nothing when `p(**non_empty)` prints as expected. `p(**empty_hash)` prints a newline, the same as `p()`, which is what I would expect when passing no keyword arguments. ---------------------------------------- Bug #16519: pp [Hash.ruby2_keywords_hash({})] shows `[nil]` https://bugs.ruby-lang.org/issues/16519#change-84031 * Author: Eregon (Benoit Daloze) * Status: Closed * Priority: Normal * Assignee: * Target version: * ruby -v: ruby 2.8.0dev (2020-01-21T13:45:10Z master 5798d35ff6) [x86_64-linux] * Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: REQUIRED ---------------------------------------- This happens on `master`: ``` $ ruby -ve 'ruby2_keywords def flag(*a); a.last; end; pp [flag(**{})]' ruby 2.8.0dev (2020-01-21T13:45:10Z master 5798d35ff6) [x86_64-linux] [nil] ``` Of course it should be `[{}]`, as it is for `pp [{}]`. On 2.7.0 it warns (should be fixed, it's valid to `pp` a flagged Hash): ``` $ ruby -ve 'ruby2_keywords def flag(*a); a.last; end; pp [flag(**{})]' ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-linux] [/home/eregon/.rubies/ruby-2.7.0/lib/ruby/2.7.0/pp.rb:226: warning: Passing the keyword argument as the last hash parameter is deprecated /home/eregon/.rubies/ruby-2.7.0/lib/ruby/2.7.0/pp.rb:334: warning: The called method is defined here {}] ``` The warning being in the middle of the output is a fun fact here. Lines it refers to (still the same on current master): https://github.com/ruby/ruby/blob/v2_7_0/lib/pp.rb#L226 https://github.com/ruby/ruby/blob/v2_7_0/lib/pp.rb#L334 This is very confusing as it can happen during `test-all` and then show output such as: ``` <[{:a=>1}]> expected but was <[{:a=>1}, nil]>. ``` when the reality is (can be verified with `p` before the `assert_equal`): ``` <[{:a=>1}]> expected but was <[{:a=>1}, {}]>. ``` -- https://bugs.ruby-lang.org/ Unsubscribe: