Project

General

Profile

Feature #14183

"Real" keyword argument

Added by mame (Yusuke Endoh) over 1 year ago. Updated 7 days ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
[ruby-core:84255]

Description

In RubyWorld Conference 2017 and RubyConf 2017, Matz officially said that Ruby 3.0 will have "real" keyword arguments. AFAIK there is no ticket about it, so I'm creating this (based on my understanding).

In Ruby 2, the keyword argument is a normal argument that is a Hash object (whose keys are all symbols) and is passed as the last argument. This design is chosen because of compatibility, but it is fairly complex, and has been a source of many corner cases where the behavior is not intuitive. (Some related tickets: #8040, #8316, #9898, #10856, #11236, #11967, #12104, #12717, #12821, #13336, #13647, #14130)

In Ruby 3, a keyword argument will be completely separated from normal arguments. (Like a block parameter that is also completely separated from normal arguments.)
This change will break compatibility; if you want to pass or accept keyword argument, you always need to use bare sym: val or double-splat ** syntax:

# The following calls pass keyword arguments
foo(..., key: val)
foo(..., **hsh)
foo(..., key: val, **hsh)

# The following calls pass **normal** arguments
foo(..., {key: val})
foo(..., hsh)
foo(..., {key: val, **hsh})

# The following method definitions accept keyword argument
def foo(..., key: val)
end
def foo(..., **hsh)
end

# The following method definitions accept **normal** argument
def foo(..., hsh)
end

In other words, the following programs WILL NOT work:

# This will cause an ArgumentError because the method foo does not accept keyword argument
def foo(a, b, c, hsh)
  p hsh[:key]
end
foo(1, 2, 3, key: 42)

# The following will work; you need to use keyword rest operator explicitly
def foo(a, b, c, **hsh)
  p hsh[:key]
end
foo(1, 2, 3, key: 42)

# This will cause an ArgumentError because the method call does not pass keyword argument
def foo(a, b, c, key: 1)
end
h = {key: 42}
foo(1, 2, 3, h)

# The following will work; you need to use keyword rest operator explicitly
def foo(a, b, c, key: 1)
end
h = {key: 42}
foo(1, 2, 3, **h)

I think here is a transition path:

  • Ruby 2.6 (or 2.7?) will output a warning when a normal argument is interpreted as keyword argument, or vice versa.
  • Ruby 3.0 will use the new semantics.

Related issues

Related to Backport200 - Backport #8040: Unexpect behavior when using keyword argumentsClosed03/08/2013Actions
Related to Ruby trunk - Bug #8316: Can't pass hash to first positional argument; hash interpreted as keyword argumentsClosedActions
Related to Ruby trunk - Bug #9898: Keyword argument odditiesClosed06/03/2014Actions
Related to Ruby trunk - Bug #10856: Splat with empty keyword args gives unexpected resultsClosedActions
Related to Ruby trunk - Bug #11236: inconsistent behavior using ** vs hash as method parameterOpenActions
Related to Ruby trunk - Bug #11967: Mixing kwargs with optional parameters changes way method parameters are parsedRejectedActions
Related to Ruby trunk - Bug #12717: Optional argument treated as kwargOpenActions
Related to Ruby trunk - Bug #12821: Object converted to Hash unexpectedly under certain method callClosedActions
Related to Ruby trunk - Bug #13336: Default Parameters don't workClosedActions
Related to Ruby trunk - Bug #13647: Some weird behaviour with keyword argumentsOpenActions
Related to Ruby trunk - Bug #14130: Keyword arguments are ripped from the middle of hash if argument have default valueOpenActions
Related to Ruby trunk - Bug #15078: Hash splat of empty hash should not create a positional argument.OpenActions

History

#1

Updated by hsbt (Hiroshi SHIBATA) over 1 year ago

  • Related to Backport #8040: Unexpect behavior when using keyword arguments added
#2

Updated by hsbt (Hiroshi SHIBATA) over 1 year ago

  • Related to Bug #8316: Can't pass hash to first positional argument; hash interpreted as keyword arguments added
#3

Updated by hsbt (Hiroshi SHIBATA) over 1 year ago

  • Related to Bug #9898: Keyword argument oddities added
#4

Updated by hsbt (Hiroshi SHIBATA) over 1 year ago

  • Related to Bug #10856: Splat with empty keyword args gives unexpected results added
#5

Updated by hsbt (Hiroshi SHIBATA) over 1 year ago

  • Related to Bug #11236: inconsistent behavior using ** vs hash as method parameter added
#6

Updated by hsbt (Hiroshi SHIBATA) over 1 year ago

  • Related to Bug #11967: Mixing kwargs with optional parameters changes way method parameters are parsed added
#7

Updated by hsbt (Hiroshi SHIBATA) over 1 year ago

  • Related to Bug #12104: Procs keyword arguments affect value of previous argument added
#8

Updated by hsbt (Hiroshi SHIBATA) over 1 year ago

  • Related to Bug #12717: Optional argument treated as kwarg added
#9

Updated by hsbt (Hiroshi SHIBATA) over 1 year ago

  • Related to Bug #12821: Object converted to Hash unexpectedly under certain method call added
#10

Updated by hsbt (Hiroshi SHIBATA) over 1 year ago

  • Related to Bug #13336: Default Parameters don't work added
#11

Updated by hsbt (Hiroshi SHIBATA) over 1 year ago

  • Related to Bug #13647: Some weird behaviour with keyword arguments added
#12

Updated by hsbt (Hiroshi SHIBATA) over 1 year ago

  • Related to Bug #14130: Keyword arguments are ripped from the middle of hash if argument have default value added

Updated by jeremyevans0 (Jeremy Evans) over 1 year ago

For a method definition like:

def foo(hsh={})
end

Will either of the following continue to work?:

foo(key: val)
foo(:key => val)

One performance issue with keyword arguments is that keyword splats allocate a hash per splat, even if no keywords are used.

In performance sensitive code, allocations can be avoided using a shared frozen hash as the default argument:

OPTS = {}.freeze
def foo(hsh=OPTS)
  bar(1, hsh)
end
def bar(val, hsh=OPTS)
end

By doing this, calling foo without keyword arguments does not allocate any hashes even if the hash is passed to other methods. If you use keyword arguments, you have to do:

def foo(**hsh)
  bar(1, **hsh)
end
def bar(val, **hsh)
end

Which I believe allocates a multiple new hashes per method call, one in the caller and one in the callee. Example:

require 'objspace'
GC.start
GC.disable
OPTS = {}

def hashes
  start = ObjectSpace.count_objects[:T_HASH]
  yield
  ObjectSpace.count_objects[:T_HASH] - start - 1
end

def foo(opts=OPTS)
  bar(opts)
end
def bar(opts=OPTS)
  baz(opts)
end
def baz(opts=OPTS)
end

def koo(**opts)
  kar(**opts)
end
def kar(**opts)
  kaz(**opts)
end
def kaz(**opts)
end

p hashes{foo}
p hashes{foo(OPTS)}
p hashes{koo}
p hashes{koo(**OPTS)}

# Output
0
0
5
6

I humbly request that unless keyword splats can be made to avoid allocation, then at least make:

def foo(hsh)
end
foo(:key => val)

still function as it has since ruby 1.8, since that can be considered a hash and not a keyword argument.

#14

Updated by mame (Yusuke Endoh) over 1 year ago

  • Backport deleted (2.3: UNKNOWN, 2.4: UNKNOWN)
  • Tracker changed from Bug to Feature

Updated by sos4nt (Stefan Schüßler) about 1 year ago

I've filed a bug report some time ago, maybe you could add it as a related issue: https://bugs.ruby-lang.org/issues/11993

Updated by dsferreira (Daniel Ferreira) about 1 year ago

It’s not clear for me all the implications of this change.
Would it be possible to exemplify the before and after behaviours in the description?

It feels to me that with this implementation it would be possible to consider both symbols and strings as keys for the keywords hash.

Would it be a possibility?

The dynamic generation of keywords hashes would be positively impacted with that move.

#17

Updated by mame (Yusuke Endoh) 8 months ago

  • Description updated (diff)

Updated by mame (Yusuke Endoh) 8 months ago

jeremyevans0 (Jeremy Evans) wrote:

For a method definition like:

def foo(hsh={})
end

Will either of the following continue to work?:

foo(key: val)
foo(:key => val)

No, it will not work. You need to rewrite the definition to def foo(**hsh).

One performance issue with keyword arguments is that keyword splats allocate a hash per splat, even if no keywords are used.

If the issue really matters, it can be fixed by lazy Hash allocation, like block parameters (#14045).

dsferreira (Daniel Ferreira) wrote:

It’s not clear for me all the implications of this change.
Would it be possible to exemplify the before and after behaviours in the description?

Added.

It feels to me that with this implementation it would be possible to consider both symbols and strings as keys for the keywords hash.

It is a completely different topic, and I'm strongly negative against allowing strings as a key.

Updated by mame (Yusuke Endoh) 8 months ago

Sorry, it seems my original description was unclear. I think it can be rephased very simply:

  • keyword argument MUST be always received as a keyword parameter
  • non-keyword argument MUST be always received as a non-keyword parameter

The following behavior will be abandoned:

  • keyword argument is passed to a last normal parameter
  • last normal hash argument is passed to keyword parameters

Here is an experimental patch to warn a deprecated behavior of keyword arguments, and it shows some OK/NG samples.

NG: a keyword argument is passed to a normal parameter

$ ./miniruby -w -e '
def foo(h)
end
foo(k: 1)
'
-e:4: warning: The keyword argument for `foo' is used as the last parameter

OK: receving it as a keyword rest argument

$ ./miniruby -w -e '
def foo(**h)
end
foo(k: 1)
'

NG: a normal hash argument is passed to a keyword argument

$ ./miniruby -w -e '
def foo(k: 1)
end
h = {k: 42}
foo(h)
'
-e:5: warning: The last argument for `foo' is used as the keyword parameter

OK: the hash as keyword argument by using **

$ ./miniruby -w -e '
def foo(k: 1)
end
h = {k: 42}
foo(**h)
'

It still needs more work. It does not support yet methods written in C because C methods always handles keyword arguments as normal arguments.

Updated by jeremyevans0 (Jeremy Evans) 8 months ago

mame (Yusuke Endoh) wrote:

jeremyevans0 (Jeremy Evans) wrote:

For a method definition like:

def foo(hsh={})
end

Will either of the following continue to work?:

foo(key: val)
foo(:key => val)

No, it will not work. You need to rewrite the definition to def foo(**hsh).

If that was the only change, it wouldn't be a big deal. However, in addition to foo(:key => val) calls, there are also foo(hsh) calls. So all callers that pass hashes would need to change from foo(hsh) to foo(**hsh). And that also breaks if there are any non-symbol keys in the hash.

In the libraries I maintain, this will be a bigger breaking change than 1.8 -> 1.9. If the decision has already been made and there is no turning back, there should probably be deprecation warnings added for it in 2.6, anytime keywords are passed to a method that accepts a default argument, or anytime a hash is passed when keyword arguments should be used.

One performance issue with keyword arguments is that keyword splats allocate a hash per splat, even if no keywords are used.

If the issue really matters, it can be fixed by lazy Hash allocation, like block parameters (#14045).

This does really matter, excessive hash allocation has a significant negative effect on performance. In addition to all of the code churn in libraries required to support this change, users of the libraries will also have to accept a significant performance hit until there is an allocation-less way to pass keyword arguments from one methods to another.

The following behavior will be abandoned:

  • keyword argument is passed to a last normal parameter
  • last normal hash argument is passed to keyword parameters

Is it possible to abandon one of these without the other? Abandoning "last normal hash argument is passed to keyword parameters" only breaks code that uses keyword arguments. Abandoning "keyword argument is passed to a last normal parameter" (supported at least back to Ruby 1.8) breaks tons of ruby code that never used keyword arguments, just to supposedly fix problems that were caused by keyword arguments.

If keyword arguments are not part of the method definition, then what is the issue with converting keyword arguments to a hash argument?

It still needs more work. It does not support yet methods written in C because C methods always handles keyword arguments as normal arguments.

What will happen to external C extension gems that use rb_get_kwargs and rb_extract_keywords, both of which accept a hash?

Updated by mame (Yusuke Endoh) 8 months ago

Jeremy, thank you for discussing this issue seriously.

jeremyevans0 (Jeremy Evans) wrote:

If that was the only change, it wouldn't be a big deal. However, in addition to foo(:key => val) calls, there are also foo(hsh) calls. So all callers that pass hashes would need to change from foo(hsh) to foo(**hsh). And that also breaks if there are any non-symbol keys in the hash.

Yes, in the current proposal, you need to rewrite all callers that passes a hash object. Note that you can already write foo(**hsh) in caller side since 2.0 (when callee-side keyword argument was introduced). Also, I believe it is a good style because the explicit operator clarifies the intent.

I have no strong opinion whether foo(:kw => 1) should pass a normal hash argument or be interpreted as keyword argument. I think the latter is better in terms of compatibility, but I'm not sure.

If keyword arguments are not part of the method definition, then what is the issue with converting keyword arguments to a hash argument?

I have never thought of this. I want to reject the following program,

def foo(*ary)
end
foo(kw: 1)

but it might be a good idea as a measure for compatibility.

It still needs more work. It does not support yet methods written in C because C methods always handles keyword arguments as normal arguments.

What will happen to external C extension gems that use rb_get_kwargs and rb_extract_keywords, both of which accept a hash?

Yes, we need to prepare C API. Ko1 has had a big plan about this since last year (or older).

Updated by jeremyevans0 (Jeremy Evans) 8 months ago

Here's an alternative proposal, with the basic idea that behavior for historical ruby 1.6+ code that doesn't use keyword arguments remains the same.

OK: Historical ruby 1.6+ (maybe before) usage (hash argument with omitted braces)

def foo(h)
  # h # => {:k => 1}
end
foo(:k => 1)
foo(k: 1) # ruby 1.9+ syntax

OK: Ruby 2.0 keyword usage that will keep working

def foo(k: 1) # or foo(**h)
end
foo(:k => 1)
foo(k: 1)
foo(**{k: 1})

NG: Using ** splat as hash argument

def foo(h)
end
foo(**{k: 1})

NG: Using hash argument instead of keyword arguments

def foo(k: 1) # or foo(**h)
end
foo({k: 1})

My reasoning for this is that historical behavior for methods that do not use keyword arguments should not be broken to fix problems caused by keyword arguments. I reviewed all issues mentioned in this ticket:

#8040: method keyword arguments
#8316: method keyword arguments
#9898: method regular argument, caller uses **
#10856: method regular argument, caller uses ** on empty array
#11236: method keyword arguments
#11967: method keyword arguments
#12104: proc usage, unrelated to keyword argument vs regular argument
#12717: method keyword arguments
#12821: method keyword arguments
#13336: method keyword arguments
#13467: method keyword arguments
#14130: method keyword arguments

As you can see, all of the problems are with using keyword arguments in the method definition or with ** at the call site when a method regular argument is used. There are no issues when the method takes a regular argument and ** is not used at the call site, with the historical behavior and syntax of specifying a hash argument with omitted braces. I see no reason to break the ruby 1.6+ historical behavior when keyword arguments are not involved.

Regarding the following program mentioned by mame:

def foo(*ary)
end
foo(kw: 1)

there is a lot of historical ruby code that does:

def foo(*ary)
  options = ary.pop if ary.last.is_a?(Hash)
  # ...
end

For that reason I think it would be best if foo(kw: 1) continued to work in such cases, since there are no problems in terms of the keyword arguments being used (no keyword arguments in method definition implies argument syntax is a hash with omitted braces).

Updated by shevegen (Robert A. Heiler) 8 months ago

I don't want to write too much, so just one comment - I would also prefer foo(kw: 1)
to retain being a Hash rather than to be assumed to be a keyword argument. I think
that it may surprise people when it would become a keyword suddenly.

Updated by matz (Yukihiro Matsumoto) 8 months ago

shevegen (Robert A. Heiler) Of course, we will take plenty of time to migrate before making it a keyword.
If we made the decision, we will make it warn you first for a year or two before the actual change.

Matz.

Updated by jeremyevans0 (Jeremy Evans) 7 months ago

To give an example of how much code this would break, let's use Redmine as an example, since it runs this bug tracker. For simplicity, let's limit our analysis to the use of a single method, ActiveRecord's where method. ActiveRecord's where method uses the following API (note, no keyword arguments):

def where(opts = :chain, *rest)
  # ...
end

where is used at least 597 times in 180 files in the application, and most of these cases appear to be calls to the ActiveRecord where method. In at least 399 cases, it appears to use an inline hash argument without braces (there are additional cases where ruby 1.9 hash syntax is used), and in 11 cases it uses an inline hash argument with braces:

$ fgrep -r .where\( !(public|doc|extra) |wc -l
     597
$ fgrep -lr .where\( !(public|doc|extra) |wc -l
     180
$ fgrep -r .where\( !(public|doc|extra) | fgrep '=>' | fgrep 'where(:' |wc -l
     399
$ fgrep -r .where\( !(public|doc|extra) | fgrep 'where({' |wc -l
      11

Examples of where usage:

# Inline hash without braces
@time_entries = TimeEntry.where(:id => params[:ids]).

# Inline hash with braces
Enumeration.where({:type => type}).update_all({:is_default => false})

# Noninline hash
condition_hash = self.class.positioned_options[:scope].inject({}) do |h, column|
  h[column] = yield(column)
  h
end
self.class.where(condition_hash)

Hopefully this serves an example of how much code this would break. Remember, this is only looking at a single method. Note that omitting the braces for hashes is almost 40x more common than including the braces.

Dropping support for braceless hashes would probably break the majority of ruby applications and libraries. Consider this another plea to limit behavior changes to methods that accept keyword arguments.

Updated by mame (Yusuke Endoh) 7 months ago

Jeremy, thank you for investigating the examples. I'd like to discuss this issue at the next developers' meeting.

This is my personal current opinion: this change indeed requires users' action, however, I believe that the problem is not so significant, and that its advantage is significant.

This change seems to remind you the breaking change of character encoding in 1.9/2.0, but it was much worse than this change because the previous one was not trivial "where to fix". The site where an error occurred was often different to the site where a wrong encoding string was created.
On the other hand, this change requires very trivial fixes. By running a test suite on Ruby 2.6 or 2.7, the interpreter will "pinpoint" all usages like you showed, and warn "this method call in line XX will not work in Ruby 3.x!". Users can easily fix the issue by checking the warnings and changing either the method calls or method definition.

I agree that compatibility is important, but the current wrong design has continuously caused troubles. This fact also looks important to me. This change will fix the issue, will make the language simpler, and will make users' code more explicit and less error-prone, which will pay users' action.

Updated by jeremyevans0 (Jeremy Evans) 7 months ago

mame (Yusuke Endoh) wrote:

Jeremy, thank you for investigating the examples. I'd like to discuss this issue at the next developers' meeting.

This is my personal current opinion: this change indeed requires users' action, however, I believe that the problem is not so significant, and that its advantage is significant.

This change seems to remind you the breaking change of character encoding in 1.9/2.0, but it was much worse than this change because the previous one was not trivial "where to fix". The site where an error occurred was often different to the site where a wrong encoding string was created.

I disagree. I migrated many applications and libraries from Ruby 1.8 to Ruby 1.9 (and later to Ruby 2.6). The changes for Ruby 1.8 -> 1.9 were minimal in comparison with the impact of this change, in terms of the amount of code that needed to be modified.

On the other hand, this change requires very trivial fixes. By running a test suite on Ruby 2.6 or 2.7, the interpreter will "pinpoint" all usages like you showed, and warn "this method call in line XX will not work in Ruby 3.x!". Users can easily fix the issue by checking the warnings and changing either the method calls or method definition.

I agree that compatibility is important, but the current wrong design has continuously caused troubles. This fact also looks important to me. This change will fix the issue, will make the language simpler, and will make users' code more explicit and less error-prone, which will pay users' action.

As I've already shown earlier in this issue, all problems in issues referenced in your initial post boil down to two basic cases:

1) Where the method being called accepts keyword arguments
2) Where double splat (**) is used by the caller and the method does not accept keyword arguments

No problems have been posted where the method does not accept keyword arguments and braces are just omitted when calling the method with an inline hash. That code has not continuously caused problems, it has worked fine since at least Ruby 1.6 with basically no changes.

The keyword argument problems started occurring in Ruby 2.0 when keyword arguments were introduced, and only affected people who chose to use define methods that accepted keyword arguments or use the double splat. If you never used double splats and never defined methods that accepted keyword arguments, either to avoid the usability and performance problems with keyword arguments or to retain compatibility with ruby <2.0, then you never ran into any of these problems.

This change makes sense for methods that accept keyword arguments, and for double splat usage on hashes when the method does not accept keyword arguments. I agree that those cases are problematic and we should fix those cases in Ruby 3. I'm just requesting that the changes be limited to those cases, and not break cases where keyword arguments and double splats were never used, since those cases have never been problematic.

Ruby is a beautiful language designed for programmer happiness. Having to change all calls from where(:id=>1) to where({:id=>1}) makes the code uglier and is going to make most Ruby programmers less happy. Does this argument for explicitness lead to requiring parentheses for all method calls?

Updated by mame (Yusuke Endoh) 7 months ago

jeremyevans0 (Jeremy Evans) wrote:

Having to change all calls from where(:id=>1) to where({:id=>1}) makes the code uglier and is going to make most Ruby programmers less happy. Does this argument for explicitness lead to requiring parentheses for all method calls?

In this specific case, it looks better to change the callee side instead of the caller side: the method definition of where should receive a keyword rest argument. Of course, it still requires us change some calls of where(opt_hash) to where(**opt_hash), but I think it is better and clearer.

Updated by mame (Yusuke Endoh) 7 months ago

Here is a scenario where allowing "hash argument with omitted braces" causes a problem. Assume that we write a method "debug" which is equal to "Kernel#p".

def debug(*args)
  args.each {|arg| puts arg.inspect }
end

Passing a hash argument with omitted braces, unfortunately, works.

debug(key: 42) #=> {:key=>42}

Then, consider we improve the method to accept the output IO as a keyword parameter "output":

def debug(*args, output: $stdout)
  args.each {|arg| output.puts arg.inspect }
end

However, this change breaks the existing call.

debug(key: 42) #=> ArgumentError (unknown keyword: key)

This is too easy to break. So, what is bad? I believe that passing a hash argument as a normal last parameter is bad.

I'd like to make it safe to extend an existing method definition with a keyword parameter.

Updated by jeremyevans0 (Jeremy Evans) 7 months ago

mame (Yusuke Endoh) wrote:

jeremyevans0 (Jeremy Evans) wrote:

Having to change all calls from where(:id=>1) to where({:id=>1}) makes the code uglier and is going to make most Ruby programmers less happy. Does this argument for explicitness lead to requiring parentheses for all method calls?

In this specific case, it looks better to change the callee side instead of the caller side: the method definition of where should receive a keyword rest argument. Of course, it still requires us change some calls of where(opt_hash) to where(**opt_hash), but I think it is better and clearer.

Changing the callee side will not fix all cases. The where method supports more than just symbols keys in hashes. where('table.id'=>1) is supported, for example. Accepting a keyword args splat and then appending it to the array of arguments just decreases performance for no benefit.

It is important to realize that keyword arguments are not a substitute for hash arguments, as keyword arguments only handle a subset of what a hash argument can handle.

mame (Yusuke Endoh) wrote:

Here is a scenario where allowing "hash argument with omitted braces" causes a problem. Assume that we write a method "debug" which is equal to "Kernel#p".

def debug(*args)
  args.each {|arg| puts arg.inspect }
end

Passing a hash argument with omitted braces, unfortunately, works.

debug(key: 42) #=> {:key=>42}

Then, consider we improve the method to accept the output IO as a keyword parameter "output":

def debug(*args, output: $stdout)
  args.each {|arg| output.puts arg.inspect }
end

However, this change breaks the existing call.

Note how this problem does not occur until you add keyword arguments to the method. If you never add keyword arguments, you never run into this problem, and there are ways to add the support you want without using keyword arguments.

Are you assuming that all methods that use hash arguments will end up wanting to use keyword arguments at some point? I think that is unlikely. If keyword arguments are never added to the method in the future, then you have broken backwards compatibility now for no benefit.

You are implying it is better to certainly break tons of existing code now, to allow for a decreased possibility of breaking code later if and only if you decide to add keyword arguments.

This is too easy to break. So, what is bad? I believe that passing a hash argument as a normal last parameter is bad.

That is an opinion I do not share. I believe passing a hash argument as a normal last parameter is fine and one of the nice features that makes Ruby a beautiful language to write in. I think omitting braces for hash arguments has a natural similarity to the ability to omit parentheses for method calls, which is another Ruby feature that makes it enjoyable to write in.

I'd like to make it safe to extend an existing method definition with a keyword parameter.

Attempting to avoid backwards compatibility problems is a noble goal that I think we share. Part of that is avoiding future backwards compatibility problems. Another part of that is avoiding current backwards compatibility problems. A change that causes more current backwards compatibility problems than the future backwards compatibility problems it is designed to avoid is a step in the wrong direction, in my opinion.

Updated by mame (Yusuke Endoh) 7 months ago

jeremyevans0 (Jeremy Evans) wrote:

Changing the callee side will not fix all cases. The where method supports more than just symbols keys in hashes. where('table.id'=>1) is supported, for example. Accepting a keyword args splat and then appending it to the array of arguments just decreases performance for no benefit.

As an experiment, I'm now trying to check Ruby's existing APIs, and noticed that some methods had the issue: Kernel#spawn, JSON::GenericObject.from_hash, etc. It might be good to provide a variant of define_method for this case as a migration path:

define_last_hash_method(:foo) do |opt|
  p opt
end

foo(k: 1)     #=> {:k=>1}
foo("k"=>1) #=> {"k"=>1}

Are you assuming that all methods that use hash arguments will end up wanting to use keyword arguments at some point? I think that is unlikely. If keyword arguments are never added to the method in the future, then you have broken backwards compatibility now for no benefit.

I don't think that all methods will have keyword arguemnts eventually. However, I assume that we can never predict which methods will have.

I'd like to make it safe to extend an existing method definition with a keyword parameter.

Attempting to avoid backwards compatibility problems is a noble goal that I think we share. Part of that is avoiding future backwards compatibility problems. Another part of that is avoiding current backwards compatibility problems. A change that causes more current backwards compatibility problems than the future backwards compatibility problems it is designed to avoid is a step in the wrong direction, in my opinion.

In general, I agree. For this specific topic, however, the current spec and implementation are really a mess; the current backward compatibility problem is relatively easy to fix; the future backwards compatibility problem is hard to avoid and will become painful more and more. We should now pay the debt for the future.

But this is just my opinion. I really appreciate and respect your opinion. I'd like to tell matz your opinion as fairly as I can.

Updated by duerst (Martin Dürst) 7 months ago

jeremyevans0 (Jeremy Evans) wrote:

mame (Yusuke Endoh) wrote:

This change seems to remind you the breaking change of character encoding in 1.9/2.0, but it was much worse than this change because the previous one was not trivial "where to fix". The site where an error occurred was often different to the site where a wrong encoding string was created.

I disagree. I migrated many applications and libraries from Ruby 1.8 to Ruby 1.9 (and later to Ruby 2.6). The changes for Ruby 1.8 -> 1.9 were minimal in comparison with the impact of this change, in terms of the amount of code that needed to be modified.

I think the amount of changes from Ruby 1.8 to Ruby 1.9 depended a lot on what kind of processing your application did, and what kind of data was involved. If you mostly just worked with US-ASCII data, the changes needed were minimal. For other data, in particular also for Japanese data, some kinds of processing may have been heavily affected.

Updated by Eregon (Benoit Daloze) 7 months ago

I agree with Jeremy here, the current idea seems too incompatible and will require too many changes (no matter the gain).
And those changes cannot easily be automated either, they need careful considerations.

I think we need to compromise here, to avoid too many incompatible changes, especially on methods which have no keyword arguments and where the intention is clear.
I would think the number of methods like debug() is a tiny fraction of the number of places we'd need to change if hash-without-braces is no longer supported.

IMHO such a method with rest + kwargs seems a bad design in the first place as the arguments are too complex. That debug method could only accept one argument for instance.

Also, how should foo(1, "foo" => "bar") behave?
Should it be like foo(1, {"foo" => "bar"})? In this case the syntax is inconsistent with foo(1, foo: "bar") where having or leaving out the braces matter.
Or does the => imply the braces?
I believe all Rubyists are used to foo(1, :foo => "bar") and foo(1, foo: "bar") being identical.

BTW, p foo: 1 will no longer work then, and p({foo: 1}) would be required, which feels very unlike Ruby, and is just impractical when debugging.

Updated by mame (Yusuke Endoh) 7 months ago

Eregon (Benoit Daloze) wrote:

I would think the number of methods like debug() is a tiny fraction of the number of places we'd need to change if hash-without-braces is no longer supported.

IMHO such a method with rest + kwargs seems a bad design in the first place as the arguments are too complex. That debug method could only accept one argument for instance.

I think you are too familiar with the current weird keyword arguments. The original and primary purpose of keyword arguments is an extension of existing methods. It looks rather "too complex" for a mere addition of keyword parameters to disturb other parameters and to break existing calls.

That being said, I agree that the "cancer" of this issue is a combination of rest/optional agruments and keyword ones. Another, more modest idea that I have is, to prohibit (or just warn) a method definition that has both rest/optional + keyword parameters. I don't like this because this spoils the purpose of keyword arguments, though.

Also, how should foo(1, "foo" => "bar") behave?
Should it be like foo(1, {"foo" => "bar"})?

I think so.

In this case the syntax is inconsistent with foo(1, foo: "bar") where having or leaving out the braces matter.

Braced hash and bare one are inconsistent, even in the current spec.

def foo(v=:default)
  p v
end

h={}
foo( **h )    #=> {}
foo({**h})    #=> {}
foo(1,  **h ) #=> 1
foo(1, {**h}) #=> wrong number of arguments (given 2, expected 0..1)

Note that **{} does not simply mean "no argument". If it was "no argument", the above foo(**h) would print :default instead of {}.

Or does the => imply the braces?
I believe all Rubyists are used to foo(1, :foo => "bar") and foo(1, foo: "bar") being identical.

They will be still identical because it is determined not only syntactically but also dynamically: a key-value pair whose key is a Symbol, is handled as keyword argument. This behavior is not new. Ruby 2.5 even does it:

def foo(h1=nil, **h2)
  p [h1, h2]
end
foo("foo" => 1, :bar => 2, baz: 3) #=> [{"foo"=>1}, {:bar=>2, :baz=>3}]

(This behavior has been changed in trunk, but I'm unsure if it is determined or not.)

BTW, p foo: 1 will no longer work then, and p({foo: 1}) would be required, which feels very unlike Ruby, and is just impractical when debugging.

I completely agree with this. I showed the method debug as an example, but I don't think that Kernel#p itself should change. Some existing APIs that people expect to accept both foo(k:1) and foo({k:1}), e.g, ERB#result_with_hash, Sequel's where, should be kept as well.

Updated by mame (Yusuke Endoh) 7 months ago

mame (Yusuke Endoh) wrote:

BTW, p foo: 1 will no longer work then, and p({foo: 1}) would be required, which feels very unlike Ruby, and is just impractical when debugging.

I completely agree with this. I showed the method debug as an example, but I don't think that Kernel#p itself should change. Some existing APIs that people expect to accept both foo(k:1) and foo({k:1}), e.g, ERB#result_with_hash, Sequel's where, should be kept as well.

Half-joking: I'm not fully satisfied with p foo: 1 #=> {:foo=>1}. If a keyword argument is separated from other ones, it can emit a much better output:

def p(*args, **kw)
  args.each {|arg| puts arg.inspect }
  kw.each {|label, arg| puts "#{ label }: #{ arg.inspect }" }
end

p foo: 1, bar: {"A"=>"B"}, baz: {qux: 1}
#=> foo: 1
#   bar: {"A"=>"B"}
#   baz: {:qux=>1}
#36

Updated by marcandre (Marc-Andre Lafortune) 7 months ago

  • Related to deleted (Bug #12104: Procs keyword arguments affect value of previous argument)

Updated by marcandre (Marc-Andre Lafortune) 7 months ago

mame (Yusuke Endoh) wrote:

Braced hash and bare one are inconsistent, even in the current spec.

def foo(v=:default)
  p v
end

h={}
foo( **h )    #=> {}
foo({**h})    #=> {}
foo(1,  **h ) #=> 1
foo(1, {**h}) #=> wrong number of arguments (given 2, expected 0..1)

The fact that foo(**h) #=> {} is a bug. Note that foo(**{}) # => :default, as I believe it should. Both should have same result. See #15078.

def foo(h1=nil, **h2)
  p [h1, h2]
end
foo("foo" => 1, :bar => 2, baz: 3) #=> [{"foo"=>1}, {:bar=>2, :baz=>3}]

I believe this is currently a bug (#14130) and I hope this is not accepted in the future either. Is there a good use case for this anyways? I fear it only creates hard to find errors.

Updated by marcandre (Marc-Andre Lafortune) 7 months ago

Let me add my voice to that of Benoit and Jeremy: the incompatibility is absolutely not worth it.

I believe that if we fix the few remaining corner cases, improve the error messages and explicitly document how Ruby handles keyword parameters vs optional positional parameters, we'll have a really solid solution.

#39

Updated by mame (Yusuke Endoh) 7 months ago

  • Related to Bug #15078: Hash splat of empty hash should not create a positional argument. added

Updated by marcandre (Marc-Andre Lafortune) 7 months ago

After working a lot on **{}, I still strongly believe that we must maintain conversion of keyword arguments to positional argument, e.g.:

def foo(*ary)
end
foo(kw: 1) # => must remain ok

OTOH, it may be possible to disallow promotion of last positional argument to keyword arguments without causing as huge incompatiblities. Using **hash could be required, if given enough time (say warnings in Ruby 2.6 & 2.7)

def foo(**options); end
foo(hash) # => Could be disallowed, only foo(**hash) would work

A major consequence of disallowing promotion to keyword arguments is that the naive forwarding calls (only with *args) will no longer be always valid. This means that all forwarding calls, including those of delegate library, will have to become capture arguments with *args, **options. This means that the meaning of **{} will become more important than it currently is.

As I argue in #15078, it will be important that **{} doesn't create a positional argument so that full forwarding works even for normal methods.

My recommendation:

Ruby 2.6: Fix **{} to not create positional argument (#15078). Improve wording of ArgumentErrors

If we want to have stricter keyword arguments (I'm not sure it's worth it), then:

Ruby 2.6: In verbose mode, warn about promotion of positional argument to keyword arguments, recommending using hash splat.
Ruby 2.7: Same, even if not-verbose.
Ruby 3.0: Stop promoting normal argument to keyword argument.

.... after I'm long dead

Ruby 42.0: Stop demoting keyword argument to normal argument

Updated by akr (Akira Tanaka) 6 months ago

I have an idea to separate positional arguments and keyword arguments without incompatibility.

Basic idea is introducing an flag, keyword_given,
which means the last argument is a Hash object which represent keyword argument.
(The name, keyword_given, is inspired from block_given? method.)

The flag will be true if method call uses k => v, k: v, "k": v or **h and
all keys of the Hash object constructed from them are symbol.
(I think hash separation is not good idea.)

The flag is referenced by a new Ruby method (keyword_given?) and
C level function (rb_keyword_given_p).

This doesn't break anything because it just add new method (and new C function).

This makes the confusion of positional/keyword arguments solvable.
But I don't say the confusion is solvable easily (or by-default).
Programmers must use the flag carefully.

If we want to solve the confusion by default, we need to change
method invocation behavior incompatible way.
However positional/keyword separation by the flag makes possible to
change behavior incrementally.

If a method is changed to use keyword_given?,
only the method is changed.
We can discuss the situation about the actual method.

If Ruby-level method definition/invocation behavior is changed
(def m(h) end cannot receive m(:k=>0) for example),
it affects many applications.
However method definition/invocation behavior contains
several points which can refer the flag.
We can discuss how big/small incompatibility and
how big/small benefits for each one.

Updated by jeremyevans0 (Jeremy Evans) 6 months ago

akr (Akira Tanaka) wrote:

I have an idea to separate positional arguments and keyword arguments without incompatibility.

I like this idea of allowing per-method handling of arguments. Just to confirm my understanding of the proposal:

def m(*a)
  [keyword_given?, a]
end

m # => [false, []]
m(1) # => [false, [1]]
m({:a=>1}) # => [false, [{:a=>1}]]
m(:a=>1) # => [true, [{:a=>1}]]
m(a: 1) # => [true, [{:a=>1}]]
m("a": 1) # => [true, [{:a=>1}]]
m(**{a: 1}) # => [true, [{:a=>1}]] or ArgumentError ?
m(**{}) # => [true, [{}]], [true, []], or ArgumentError ?
m('a'=>1, :a=>1) # => [false, [{'a'=>1, :a=>1}]]
a = :a
m(a=>1) # => [true, [{:a=>1}]]

def m2(*a, **kw)
  [keyword_given?, a, kw]
end

m2 # => [false, [], {}]
m2(1) # => [false, [1], {}]
m2({:a=>1}) # => [false, [{:a=>1}], {}]
m2(:a=>1) # => [true, [], {:a=>1}]
m2(a: 1) # => [true, [], {:a=>1}]
m2("a": 1) # => [true, [], {:a=>1}]
m2(**{a: 1}) # => [true, [], {:a=>1}]
m2(**{}) # => [true, [], {}]
m2('a'=>1, :a=>1) # => [false, [{'a'=>1, :a=>1}], {}]
a = :a
m2(a=>1) # => [true, [], {:a=>1}]

def m3(a)
  [keyword_given?, a]
end

m3 # => ArgumentError
m3(1) # => [false, 1]
m3({:a=>1}) # => [false, {:a=>1}]
m3(:a=>1) # => [true, {:a=>1}]
m3(a: 1) # => [true, {:a=>1}]
m3("a": 1) # => [true, {:a=>1}]
m3(**{a: 1}) # => [true, {:a=>1}] or ArgumentError ?
m3(**{}) # => [true, {}] or ArgumentError ?
m3('a'=>1, :a=>1) # => [false, {'a'=>1, :a=>1}]
a = :a
m3(a=>1) # => [true, {:a=>1}]

def m4(**kw)
  [keyword_given?, kw]
end

m4 # => [true, {}] or [false, {}] ?
m4(1) # => ArgumentError
m4({:a=>1}) # => ArgumentError
m4(:a=>1) # => [true, {:a=>1}]
m4(a: 1) # => [true, {:a=>1}]
m4("a": 1) # => [true, {:a=>1}]
m4(**{a: 1}) # => [true, {:a=>1}]
m4(**{}) # => [true, {}] or [false, {}] ?
m4('a'=>1, :a=>1) # => ArgumentError
a = :a
m4(a=>1) # => [true, {:a=>1}]

If the method does not explicitly declare any keyword arguments, and the caller uses **hash is used to explicitly pass a keyword argument, does that raise an ArgumentError or does it pass the hash as a positional argument and have keyword_given? return true?

If the method does not explicitly declare any keyword arguments, does it pass the splatted empty hash as a positional argument, does it ignore it, or does it raise an ArgumentError?

If the method explicitly declares keyword arguments (either required keyword, optional keyword, or keyword splat), and is called without keyword arguments, does keyword_given? return true or false?

If the method explicitly declares keyword arguments and an empty hash is splatted, does keyword_given? return true or false?

Updated by akr (Akira Tanaka) 6 months ago

keyword_given? provides information about the caller side.
The information is not related to callee side.
There is no chance to call keyword_given? if ArgumentError is raised, though.

For simplicity, I assume keys for keyword argument is Symbol, here.
(If non-Symbol key is provided, keyword_given? returns false.)

jeremyevans0 (Jeremy Evans) wrote:

If the method does not explicitly declare any keyword arguments, and the caller uses **hash is used to explicitly pass a keyword argument, does that raise an ArgumentError or does it pass the hash as a positional argument and have keyword_given? return true?

keyword_given? return true.
We can discuss ArgumentError or not.

If the method does not explicitly declare any keyword arguments, does it pass the splatted empty hash as a positional argument, does it ignore it, or does it raise an ArgumentError?

If splatted empty hash means **{} in caller side, keyword_given? return true.
We can discuss ArgumentError or not.

If the method explicitly declares keyword arguments (either required keyword, optional keyword, or keyword
splat), and is called without keyword arguments, does keyword_given? return true or false?

keyword_given? return false.
I think there is no chance for keyword argument related ArgumentError if no required keyword.

If the method explicitly declares keyword arguments and an empty hash is splatted, does keyword_given? return true or false?

keyword_given? return true.

I assume **{} in caller add {} in arguments and keyword_given? return true.
However, another behavior is possible: **{} doesn't add {} in arguments and keyword_given? return false.
Their difference is visible until we have a way to obtain positional arguments and keyword argument in single array.
I choose former because I'm considering to distinguish them using **nil and `**{}'.
See details with https://bugs.ruby-lang.org/issues/15078#note-13

Updated by marcandre (Marc-Andre Lafortune) 6 months ago

Very interesting.

akr (Akira Tanaka) wrote:

The flag will be true if method call uses k => v, k: v, "k": v or **h and
all keys of the Hash object constructed from them are symbol.
(I think hash separation is not good idea.)

I agree that hash separation is not a good idea. I'm wondering if (k = :a) => v should be accepted. It is the only case that is not syntactical.

This makes the confusion of positional/keyword arguments solvable.
But I don't say the confusion is solvable easily (or by-default).
Programmers must use the flag carefully.

IIUC, the only 100% correct way to forward a method call (including the keyword_given? flag) would be:

def forward(*args, &block)
  if keyword_given?
    options = args.pop
    target(*args, **options, &block)
  else
    target(*args, &block)
  end
end

That is assuming the current **{} creating a positional argument.

Assuming Ruby 2.x compatiblity, there's no way of a general forward with **capture though. We'd need a method lash_argument_converted_to_keyword?...

def forward(*args, **options, &block)
  if keyword_given?
    target(*args, **options, &block)
  else
    args << options if last_argument_converted_to_keyword?
    target(*args, &block)
  end
end

Maybe the API could be combined in a single method keyword_style, returning one of [nil, :keyword, :hash_to_keyword, :keyword_to_hash]:

def foo(*); keyword_style; end
def bar(**); keyword_style; end
h = {}

foo      # => nil
foo(**h) # => :keyword_to_hash
bar(**h) # => :keyword
bar(h)   # => :hash_to_keyword

Or keyword_given? could return nil in case a hash was converted to a keyword argument like in bar(h)

If we distinguish **nil and **{} as in #15078, then there's no need to even call keyword_given? and normal forwarding works I imagine... But that might be quite incompatible.

Updated by mame (Yusuke Endoh) 4 months ago

I talked with matz about this proposal at Keep Ruby Weird conference (more precisely, Franklin BBQ at Austin). As far as I understand, Matz currently likes syntactical separation of keyword and normal arguments. (Note that it is not decided yet.)

Short summary:

def foo(**kw); p kw; end
def bar(kw = {}); p kw; end
h = {:k => 1}

# base (non-braced) hash arguments passed as keywords
foo(k: 1)    #=> {:k=>1} in 2.X and 3.0
foo(:k => 1) #=> {:k=>1} in 2.X and 3.0
foo(**h)     #=> {:k=>1} in 2.X and 3.0
bar(k: 1)    #=> {:k=>1} in 2.X, ArgumentError in 3.0
bar(:k => 1) #=> {:k=>1} in 2.X, ArgumentError in 3.0
bar(**h)     #=> {:k=>1} in 2.X, ArgumentError in 3.0

# braced hash arguments are passed as a last argument
foo({ k: 1 })    #=> {:k=>1} in 2.X, ArgumentError in 3.0
foo({ :k => 1 }) #=> {:k=>1} in 2.X, ArgumentError in 3.0
foo(h)           #=> {:k=>1} in 2.X, ArgumentError in 3.0
bar({ k: 1 })    #=> {:k=>1} in 2.X and 3.0
bar({ :k => 1 }) #=> {:k=>1} in 2.X and 3.0
bar(h)           #=> {:k=>1} in 2.X and 3.0

Unfortunately, this change will break many existing programs. But, it would be still easy to fix. We can pick up keywords or normal hash explicitly for each callsite that an error occurred. In many cases, keywords would be preferable: just change from def foo(h = {}) to def foo(**h), and from foo(h) to foo(**h).

And, this is a new topic. There is an non-Symbol-key call, like where("table.id" => 1) (which was shown by Jeremy Evans). This is difficult to change to keyword argument. To allow this, matz came up with an idea: non-Symbol key is also allowed as a keyword.

def foo(**kw)
  p kw
end

foo("str" => 42) #=> {"str"=>42}

Note that, if you need to write a library that works on both 2.X and 3.X, you must write a shim:

def foo(kw1 = {}, **kw2)
  kw = kw1.merge(kw2)
  kw
end

However, after EOL of all Ruby 2.X series, you can remove the shim and just write a simple code. This is better than my original proposal.

What do you think?

Updated by jeremyevans0 (Jeremy Evans) 4 months ago

mame (Yusuke Endoh) wrote:

And, this is a new topic. There is an non-Symbol-key call, like where("table.id" => 1) (which was shown by Jeremy Evans). This is difficult to change to keyword argument. To allow this, matz came up with an idea: non-Symbol key is also allowed as a keyword.

def foo(**kw)
  p kw
end

foo("str" => 42) #=> {"str"=>42}

Note that, if you need to write a library that works on both 2.X and 3.X, you must write a shim:

def foo(kw1 = {}, **kw2)
  kw = kw1.merge(kw2)
  kw
end

However, after EOL of all Ruby 2.X series, you can remove the shim and just write a simple code. This is better than my original proposal.

What do you think?

I agree with the proposed changes to foo-like (keyword arguments) methods, as those changes actually solve real problems with keyword arguments.

I disagree with the proposed changes to bar-like (positional hash arguments) methods. Those changes do not solve existing problems, they just break existing code for the potential future ability to introduce keyword arguments without behavior changes.

Let's consider if this proposed changes to bar-like (positional hash arguments) methods is accepted. The main argument for acceptance is the ability to introduce keyword arguments without behavior changes. However, in many if not most cases where bar-like methods are used, keyword arguments would be used to replace the option hashes (something that works OK in 2.X except for corner cases with optional position arguments and argument splats), not as an addition to option hashes. With the changes discussed to foo-like methods, you would no longer be able to replace an option hash argument with keyword arguments in a backwards compatible manner. So the fact that the proposed changes to bar-like methods allow keyword arguments to be introduced in a backwards compatible manner will not help, since replacing the option hashes with keywords will still be a backwards incompatible change.

There are many cases where you have a method that accepts a hash where you have cases where you want to pass an existing hash and other cases where you want pass a new hash, and having to add braces to all call-sites where you currently can omit them would be annoying, add no value, and make the code slightly harder to read.

The performance disadvantages to keyword splats that I discussed earlier still have not be addressed, and it is still impossible to create a method that accepts arbitrary keyword arguments and delegates the call to another method that accepts arbitrary keyword arguments without at least 3 hash allocations per-call (and you can have 0 hash allocations per call with a option-hash based approach).

In conclusion, the ability to add keyword arguments in a backwards compatible manner to methods that accept option hashes adds very little benefit. I think there are huge costs in breaking existing compatibility (potentially leading to a Python 2/3-like situation in libraries), and other costs in making code using bar-like methods harder to read (by requiring braces), as well as hurting performance by encouraging unoptimized keyword splats as a replacement for option hashes.

The ability for **kw to accept non-Symbol keys would make it a slightly easier to convert option hash methods to keyword arguments methods, but the keyword argument approach would still perform worse due to the additional hash allocations, and converting option hashes to keyword splats would still not be backwards compatible, and I think in most cases using an options hash would still be the preferable approach. So even if **kw handled non-Symbol keys, I would still be strongly against changing the behavior for bar-like methods.

Updated by decuplet (Nikita Shilnikov) 3 months ago

I'm not sure if this was discussed but one more thing to consider is hash destructuring using keywords. As far as I understand there's a use case I rely on which is going to be broken by the proposed changes. Specifically, things like

xs = [a: 1, b: 2, c: 3]
xs.map { |a:, b:, c:| ... }

Perhaps it makes sense to make hash destructuring a separate feature with the following syntax

xs.map { |{a, b, c}| ... }
xs.map { |{a, **rest}| ... }

But it seems to be a separate feature and I don't see how this can help with other compatibility issues mentioned here.

Updated by ioquatix (Samuel Williams) 3 months ago

I agree we should fix this issue. It is very much unexpected behaviour and even context sensitive behaviour. Double splat operator should be required in all cases to turn hash into keyword arguments.

Updated by mame (Yusuke Endoh) 7 days ago

Sorry for leaving this ticket. Matz, akr and I talked about this issue several times since the last year, and we have never reached a perfect solution. But I try to re-summarize the problem, current proposal, and migration path.

Problem

The current spec of keyword arguments is broken in several senses.

1. Keyword extension is not always safe

We call "keyword extension" to add a keyword parameter to an existing method.
Unfortunately, keyword extension is not safe when the existing method accepts rest arguments.

def foo(*args)
  p args
end
foo(key: 42) #=> [{:key=>42}]

If we add a new mode to the method, the existing call will break.

def foo(*args, output: $stdout)
  output.puts args.inspect
end
foo(key: 42) #=> unknown keyword: key

Safe keyword extension is a fundamental expectation for keyword arguments, so that is a pity.

2. Explicit Delegation of keywords backfires

You are writing a delegation, and you think of keywords, so you wrote:

def foo(*args, **kw, &blk)
  bar(*args, **kw, &blk)
end

However, this does not work correctly.

def bar(*args)
  p args
end

foo() #=> excepted:[], actual:[{}]

3. There are many unintuitive corner cases

There are many bug reports about keyword arguments. One of the most weird cases:

def foo(opt=42, **kw)
  p [opt, kw]
end

foo({}, **{})  #=> expected:[{}, {}], actual:[42, {}]

All of these issues are caused by the fundamental design flaw of the current keyword arguments which handles a keyword as a last positional argument that is a Hash object. Matz, akr and I have considered these issues seriously. Actually, matz came up with multiple ideas that would be compatible (or mildly incompatible) and solve the issues. However, all of them were proved to be incompatible, complex, and/or not to solve some of the above issues.

Proposal for 3.X semantics

The current proposal consists of two parts:

A) Separate keyword arguments from positional arguments completely
B) Allow non-Symbol keys as a keyword

(A) is the original proposal of this ticket.

  • A keyword argument is passed only by foo(k: 1) or foo(**opt), and accepted only by def foo(k: 1) or def foo(**opt).
  • A positional Hash argument is passed only by foo({ k: 1 }) or foo(opt), and accepted only by def foo(opt) or def foo(opt={}) or def foo(*args)

See the next section in detail.

(B) allows some DSL usages of brace omission:

def where(**kw)
  p kw
end

where("table.id" => 42) #=> {"table.id"=>42}

Actually, this behavior is not new. Ruby 2.0.0-p0 allowed non-Symbol keys.

Typical rewrite cases

This change brings incompatibility, so you need to rewrite existing code. Typical rewrite cases are three (plus one):

1. Accept keywords by **opt, not by opt={}

# NG in 3.X
def foo(opt={})
end

# OK in 3.X
def foo(**opt)
end

2. Pass keywords without braces, or with explicit **

def foo(**opt)
end

# NG in 3.X
foo({ k: 1 })
h = { k: 1 }
foo(h)

# OK in 3.X
foo(k: 1)
foo(**h)

3. Delegate keyword argument explicitly

# NG in 3.X
def foo(*args, &blk)
  bar(*args, &blk)
end

# OK in 3.X
def foo(*args, **kw, &blk)
  bar(*args, **kw, &blk)
end

Plus one. Manually merge the last argument with a keyword argument

If you want to allow both calling styles, you can do it manually.

# NG in 3.X
def foo(opt={})
  p opt
end
foo({ k: 1 }) #=> {:k=>1}
foo(k: 1)     #=> expected:{:k=>1}, actual:error

# OK in 3.X
def foo(opt={}, **kw)
  opt = opt.merge(kw)
  p opt
end
foo({ k: 1 }) #=> {:k=>1}
foo(k: 1)     #=> {:k=>1}

Migration path: 2.7 semantics

Basic approach:

  • If a code is valid (no exception raised) in 3.X, Ruby 2.7 should run it in the same way as 3.X
  • If a code is invalid (an exception raised) in 3.X, Ruby 2.7 should run it in the same way as 2.6, but a warning is printed

Typical examples:

def foo(opt)
end
foo(k: 1) #=> test.rb:3: warning: The keyword argument for `foo' (defined at test.rb:1) is used as the last parameter
def foo(**opt)
end
foo({ k: 1 }) #=> test.rb:3: warning: The last argument for `foo' (defined at test.rb:1) is used as the keyword parameter

These warnings tell users how to fix the source code.

(A naive implementation of this approach is not enough. Very subtle hack is required for delegation. This is explained in the last appendix section.)

Experiment

I have implemented 2.7's candidate semantics:

https://github.com/ruby/ruby/compare/trunk...mame:keyword-argument-separation

And I actually modified the standard libraries and tests to support the keyword argument separation. Many of the changes are one of the three (plus one) typical rewrite cases. There are a few tricky modifications, but in my opinion, almost all of them were trivial.

In addition, I tested an internal Rails app in my company (about 10k lines) with my prototype. Honestly speaking, when running rake spec, it produces about 120k (!) warnings, but there are many duplicated warnings. By removing the duplications, we got about 1k warnings. And, I found that almost all warnings were produced in gems. If we focus on only the application itself, we found only five method definitions to be modified. All fixes were the first typical rewrite case: def foo(opt={}) -> def foo(**opt)). We will need to rewrite some more calls to add an explicit ** if some libraries decided that their APIs only accept keywords.

Appendix: Special frozen Hash object for delegation

Unfortunately, the naive implementation of the migration path is incomplete with regard to delegation.
Consider the following code.

# in 2.7
def f1(k: 1)
  p k
end

def f2(*args)
  p args
end

def dispatch(target, *args, &blk)
  if target == :f1
    f1(*args, &blk)
  else
    f2(*args, &blk)
  end
end

dispatch(:f1, k: 1) #=> 1
#=> t.rb:17: warning: The keyword argument for `dispatch' (defined at t.rb:9) is used as the last parameter
#   t.rb:11: warning: The last argument for `f1' (defined at t.rb:1) is used as the keyword parameter
#   1

dispatch(:f2, 1, 2, 3) #=> [1, 2, 3]

You see a warning, so you rewrite it by explicit keyword delegation:

# in 2.7
def f1(k: 1)
  p k
end

def f2(*args)
  p args
end

def dispatch(target, *args, **kw, &blk)
  if target == :f1
    f1(*args, **kw, &blk)
  else
    f2(*args, **kw, &blk)
  end
end

dispatch(:f1, k: 1)    #=> 1
dispatch(:f2, 1, 2, 3) #=> [1, 2, 3, {}]
#=> t.rb:18: warning: The keyword argument for `f2` (defined at t.rb:4) is used as the last parameter

dispatch(:f1, k: 1) works perfectly with no warnings. However, the result of dispatch(:f2, 1, 2, 3) changed and a new warning is emitted. This is because **kw was automatically converted to a positional argument (due to 2.6 compatibility layer).

To fix this issue, we introduce a Hash flag to distinguish between "no keyword given" and "empty keyword given".

def foo(**kw)
  p kw
end

foo(**{}) #=> {}
foo()     #=> {(NO KEYWORD)}

{} is a normal empty hash object, and {(NO KEYWORD)} is the special empty hash object that represents "no keyword given".

If we pass the flagged empty hash to another method with ** operator, it is omitted.

def bar(*args)
  p args
end

def foo(**kw)
  # kw is {(NO KEYWORD)}
  bar(**kw) # **{(NO KEYWORD)} is equal to nothing: bar()
end

foo({}) #=> [{}]
foo()   #=> []

This is akr's idea that was explained at https://bugs.ruby-lang.org/issues/14183#note-41.

This hack of special empty hash flag is temporal just during the migration. After 3.X completes the separation of keyword arguments, this dirty hack can be removed.

Updated by jeremyevans0 (Jeremy Evans) 7 days ago

mame,

Thanks for your continued work on this.

I still agree that for methods that accept keyword arguments, we should make changes to avoid the problems that currently exist for keyword arguments.

I still believe that breaking all backwards compatibility for methods that do not currently accept keyword arguments, just to allow keyword arguments to be added safely in the future, is not a worthy tradeoff, for the following reasons:

  • Many if not most of the methods may never be converted to keyword arguments, in which case backwards compatibility is broken for no benefit.

  • This encourages the use of keyword arguments, while the use of keyword arguments hurts performance in all cases where keyword splats are used (either at the caller side or the callee side). The option hash approach can be made faster and allocation-less, while all keyword splats are currently slower as they require allocations. I'm not sure that the keyword argument splat performance issues could be fixed without breaking backwards compatibility for all keyword argument splats.

  • For methods that currently use option hashes, requiring braces around the option hash can make it more difficult to convert to keyword arguments, not less. A method such as def foo(opts={}) end that is usually called using foo(bar: 1), will still work if you switch to keyword arguments: def foo(**opts) end. It is true that the foo(hash) calling style would require modifications with the switch to keyword arguments, though.

I think the biggest problem with keeping backwards compatibility for methods that do not accept keyword arguments is handling delegation.

def foo(*a, **kw, &block)
  bar(*a, **kw, &block)
end

I believe with your proposal, this is expected to work regardless of whether bar accepts keyword arguments. If bar doesn't accept keyword arguments, then calling foo with a keyword argument will raise an exception when foo calls bar. I think one possible way to get that simple delegation to work would be to allow double-splat when calling methods that do not accept keyword arguments (keep backwards compatibility). For example, allow this:

def bar(hash={})
  hash[:a]
end

bar(**{a: 1})
# => 1

foo(**{a: 1})
# => 1

This keeps backwards compatibility back to Ruby 2.0. It will also make it easier to transition such code to keyword arguments later without breaking backwards compatibility, since changing the definition of bar to def bar(**hash) hash[:a] end would still work in that case.

The main problematic case would be if bar accepted a positional splat but did not accept keyword arguments, where an empty hash would be provided if no keyword arguments were used:

def bar(*a)
  a
end

bar
# => []

foo
# => [{}]

One possible way around that would be that if a method accepts a positional splat and does not accept keyword arguments, then calling the method with an empty keyword argument splat would not pass a positional argument. Proposed behavior:

def bar(*a)
  a
end

bar
# => []

foo
# => []

bar(**{})
# => []

foo(**{})
# => []

bar(1, a: 1)
# => [1, {a: 1}]

foo(1, a: 1)
# => [1, {a: 1}]

My Proposed Alternative

To sum up, here is my proposed alternative approach:

  • For methods that accept keyword arguments, the same as your proposal
  • For methods that do not accept keyword arguments:
    • Allow braceless hashes as positional arguments (keep backwards compatibility)
    • Allow **keyword splats
      • If keyword is empty hash, do not add the empty hash positional argument (new behavior)
      • Otherwise, add keyword as positional hash argument (keep backwards compatibility)

I think this alternative proposal handles "2. Explicit Delegation of keywords backfires" and "3. There are many unintuitive corner cases". It does not handle "1. Keyword extension is not always safe". However, I believe you could keep safe keyword extension if using keyword splat, using an approach that works and is backwards compatible to Ruby 2.0. From your example:

# Before
def foo(*args)
  p args
end
foo(key: 42)
# => [{:key=>42}]

# Add keyword arguments
def foo(*args, output: $stdout, **kw)
  args << kw
  output.puts args.inspect
end
foo(key: 42)
# => [{:key=>42}]

Issues with keyword-argument-separation branch

In terms of the specific implementation in your keyword-argument-separation branch:

The rb_no_keyword_hash approach breaks modification of the hash, which I believe is unexpected:

def foo(**opts)
  opts
end

foo
#  => {(NO KEYWORD)}

def foo(**opts)
  opts[:a] = 1
  opts
end

foo
# FrozenError (can't modify frozen Hash)

It may be possible to work around that by setting a flag on the hash instead of using a shared frozen hash, assuming there is a spare flag we can use for that purpose. If a flag isn't available, we probably could use an instance variable that doesn't start with @ (making it only visible to C).

The warning seems inconsistent. For positional splats, you get warned if the braceless hash is the first argument, but not if it is a subsequent argument:

def bar(*a)
  a
end

bar
=> []

bar(a: 1)
# warning: The keyword argument for `bar' (defined at XXX) is used as the last parameter
# => [{:a=>1}]

bar(1, a: 1)
# => [1, {:a=>1}]

This situation also occurs for methods without splats where both arguments are optional (and maybe other cases):

def baz(a=1, b={})
  [a, b]
end

baz
# => [1, {}]

baz(a: 2)
# warning: The keyword argument for `baz' (defined at XXX) is used as the last parameter
[{:a=>2}, {}]

baz(1, a: 2)
# => [1, {:a=>2}]

Is that behavior in regards to warnings expected?

Behavior is different for methods defined in C, as C methods are always passed a hash, so the brace, braceless, and splat forms all work:

String.new(capacity: 1000)
# => ""
String.new({capacity: 1000})
# => ""
String.new(**{capacity: 1000})
# => ""

This results in inconsistent behavior depending how how the method is defined. This will lead to backwards compatibility problems if you move a method definition from C to ruby, or if you have a method defined in both C and ruby, with the pure ruby version used as a fallback if the C version cannot be used.

I look forward to discussing this issue in person at the developer meeting next month.

Also available in: Atom PDF