Project

General

Profile

Actions

Bug #20218

closed

aset/masgn/op_asgn with keyword arguments

Added by jeremyevans0 (Jeremy Evans) 4 months ago. Updated about 20 hours ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:116460]

Description

I found that use of keyword arguments in multiple assignment is broken in 3.3 and master:

h = {a: 1}
o = []
def o.[]=(*args, **kw)
  replace([args, kw])
end

# This segfaults as RHS argument is not a hash
o[1, a: 1], _ = [1, 2]


# This passes the RHS argument as keywords to the method, treating keyword splat as positional argument
o[1, **h], _ = [{b: 3}, 2]
o
# => [[1, {:a=>1}], {:b=>3}]

Before 3.3, keyword arguments were treated as positional arguments.

This is similar to #19918, but for keyword arguments instead of block arguments.

@matz (Yukihiro Matsumoto) indicated he wanted to prohibit block arguments in aset/masgn and presumably also op_asgn (making them SyntaxErrors). Can we also prohibit keyword arguments in aset/masgn/op_asgn?

Note that aset treats keyword arguments as regular arguments:

o[1, a: 1] = 2
o
# => [[1, {:a=>1}, 2], {}]

o[1, **h] = {b: 3}
o
# => [[1, {:a=>2}, {:b=>3}], {}]

While op_asgn treats keyword arguments as keywords:

h = {a: 2}
o = []
def o.[](*args, **kw)
  concat([:[], args, kw])
  x = Object.new
  def x.+(v)
    [:x, v]
  end
  x
end
def o.[]=(*args, **kw)
  concat([:[]=, args, kw])
end
o[1, a: 1] += 2
o
# => [:[], [1], {:a=>1}, :[]=, [1, [:x, 2]], {:a=>1}]

o.clear
o[1, **h] += {b: 3}
o
# => [:[], [1], {:a=>2}, :[]=, [1, [:x, {:b=>3}]], {:a=>2}]

Updated by matz (Yukihiro Matsumoto) 3 months ago

OK, prohibit keyword arguments in aset.

Matz.

Updated by kddnewton (Kevin Newton) 3 months ago

Does this also include blocks? Sorry I can't remember if that was officially decided or not. Also I'm presuming this would be for Ruby 3.4?

Updated by mame (Yusuke Endoh) 3 months ago

Does this also include blocks?

Yes. @nobu (Nobuyoshi Nakada) wrote a patch in #19918, so a block will also be prohibited.

Note that keyword arguments and a block are allowed in the normal method call style: obj.[]=(1, 2, kw: 1, &blk).

Actions #5

Updated by nobu (Nobuyoshi Nakada) 2 months ago

  • Status changed from Open to Closed

Applied in changeset git|0d5b16599a4ad606619228623299b931c48b597b.


[Bug #20218] Reject keyword arguments in index

Updated by bughit (bug hit) about 20 hours ago ยท Edited

In this issue there's no consideration of compatibility or utility. This is a breaking change. The ability to pass kwargs to index methods has been in ruby for a long time, probably from the inception of kwargs and I have code that uses that. Ruby doesn't have a spec so MRI behavior is effectively the spec so you can't say using kwargs in index methods was somehow "wrong". It wasn't "wrong" empirically and it's not "wrong" conceptually. kwargs just allow for more variability in store/lookup operations via index methods, it allows you to control where/how something is stored/looked up.

this is from 2.6

module IndexTest
  @store = {}

  def self.store
    @store
  end

  def self.key(name, namespace: nil)
    name = "#{namespace}:#{name}" if namespace
    name
  end

  def self.[](name, namespace: nil)
    p [name, namespace]
    @store[key(name, namespace: namespace)]
  end

  def self.[]=(name, opts = {}, val)
    p [name, opts, val]
    @store[key(name, namespace: opts[:namespace])] = val
  end


end

IndexTest['foo'] = 1
p IndexTest['foo']
IndexTest['foo', namespace: 'bar'] = 2
p IndexTest['foo', namespace: 'bar']
p IndexTest.store

So kwargs in index methods should be preserved for the sake of compatibility and utility. The only reasonable breaking change here is for []= to have real kwargs, rather than the middle positional kwarg collector hash in the above example.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0