[ruby-core:115430] [Ruby master Bug#20012] Fix keyword splat passing as regular argument

[ruby-core:115430] [Ruby master Bug#20012] Fix keyword splat passing as regular argument

[jeremyevans0 (Jeremy Evans) via ruby-core]

Issue #20012 has been reported by jeremyevans0 (Jeremy Evans).

----------------------------------------
Bug #20012: Fix keyword splat passing as regular argument
https://bugs.ruby-lang.org/issues/20012

* Author: jeremyevans0 (Jeremy Evans)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.3.0dev (2023-11-12 master 60e19a0b5f) [x86_64-openbsd]
* Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN
----------------------------------------
Since Ruby 3.0, Ruby has passed a keyword splat as a regular argument in the case of a call to a Ruby method where the method does not accept keyword arguments, if the method call does not contain an argument splat:

```ruby
def self.f(obj) obj end
def self.fs(*obj) obj[0] end
h = {a: 1}
f(**h).equal?(h)  # Before: true; After: false
fs(**h).equal?(h) # Before: true; After: false

a = []
f(*a, **h).equal?(h)  # Before and After: false
fs(*a, **h).equal?(h) # Before and After: false
```

The fact that the behavior differs when passing an empty argument splat makes it obvious that something is not working the way it is intended.  The fact that the hash is copied for C methods also makes it obvious.  Ruby 2 always copied the keyword splat hash, and I think that is the expected behavior in Ruby 3.

This bug is because of a missed check in setup_parameters_complex. If the keyword splat passed is not mutable, then it points to an existing object and not a new object, and therefore it must be copied.

I did not bisect to find the commit that introduced the problem, but this seems likely to be something I introduced during keyword argument separation.  I'm surprised that this has not been filed as a bug sooner.  Maybe it just rarely comes up in practice, and I only discovered it recently. Possibly, other developers that may have discovered this may have decided against submitting this as an issue, as we have had specs for two years that give the impression that the broken behavior was expected.

I've submitted a pull request to fix this: https://github.com/ruby/ruby/pull/8970



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:115431] [Ruby master Bug#20012] Fix keyword splat passing as regular argument

[matz (Yukihiro Matsumoto) via ruby-core]

Issue #20012 has been updated by matz (Yukihiro Matsumoto).


 I am in favor of this change, but at the same time concerned about compatibility. I'd like to make sure there won't be any major problems before adopting it.

Matz.


----------------------------------------
Bug #20012: Fix keyword splat passing as regular argument
https://bugs.ruby-lang.org/issues/20012#change-105361

* Author: jeremyevans0 (Jeremy Evans)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.3.0dev (2023-11-12 master 60e19a0b5f) [x86_64-openbsd]
* Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN
----------------------------------------
Since Ruby 3.0, Ruby has passed a keyword splat as a regular argument in the case of a call to a Ruby method where the method does not accept keyword arguments, if the method call does not contain an argument splat:

```ruby
def self.f(obj) obj end
def self.fs(*obj) obj[0] end
h = {a: 1}
f(**h).equal?(h)  # Before: true; After: false
fs(**h).equal?(h) # Before: true; After: false

a = []
f(*a, **h).equal?(h)  # Before and After: false
fs(*a, **h).equal?(h) # Before and After: false
```

The fact that the behavior differs when passing an empty argument splat makes it obvious that something is not working the way it is intended.  The fact that the hash is copied for C methods also makes it obvious.  Ruby 2 always copied the keyword splat hash, and I think that is the expected behavior in Ruby 3.

This bug is because of a missed check in setup_parameters_complex. If the keyword splat passed is not mutable, then it points to an existing object and not a new object, and therefore it must be copied.

I did not bisect to find the commit that introduced the problem, but this seems likely to be something I introduced during keyword argument separation.  I'm surprised that this has not been filed as a bug sooner.  Maybe it just rarely comes up in practice, and I only discovered it recently. Possibly, other developers that may have discovered this may have decided against submitting this as an issue, as we have had specs for two years that give the impression that the broken behavior was expected.

I've submitted a pull request to fix this: https://github.com/ruby/ruby/pull/8970



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:115434] [Ruby master Bug#20012] Fix keyword splat passing as regular argument

[byroot (Jean Boussier) via ruby-core]

Issue #20012 has been updated by byroot (Jean Boussier).


I tried this on Shopify's monolith CI and it's breaking quite hard:

```ruby
      ATTRIBUTE_NAMES = T.let(
        [
          :currency,
          :description,
          :kind,
          :price,
          :reasons,
          :receipt,
          :shipping_service_charge_id,
          :shipping_service_label_id,
          :origin_address,
        ].freeze,
        T::Array[Symbol]
      )

      T.unsafe(self).validates(*ATTRIBUTE_NAMES, presence: true) # here
```

```
 `<class:Payload>': can't modify frozen Array: [:currency, :description, :kind, :price, :reasons, :receipt, :shipping_service_charge_id, :shipping_service_label_id, :origin_address] (FrozenError)
	from /app/components/delivery/app/services/shipping/adjustment_creator/payload.rb:6:in `<class:AdjustmentCreator>'
	from /app/components/delivery/app/services/shipping/adjustment_creator/payload.rb:5:in `<module:Shipping>'
	from /app/components/delivery/app/services/shipping/adjustment_creator/payload.rb:4:in `<main>'
```

----------------------------------------
Bug #20012: Fix keyword splat passing as regular argument
https://bugs.ruby-lang.org/issues/20012#change-105365

* Author: jeremyevans0 (Jeremy Evans)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.3.0dev (2023-11-12 master 60e19a0b5f) [x86_64-openbsd]
* Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN
----------------------------------------
Since Ruby 3.0, Ruby has passed a keyword splat as a regular argument in the case of a call to a Ruby method where the method does not accept keyword arguments, if the method call does not contain an argument splat:

```ruby
def self.f(obj) obj end
def self.fs(*obj) obj[0] end
h = {a: 1}
f(**h).equal?(h)  # Before: true; After: false
fs(**h).equal?(h) # Before: true; After: false

a = []
f(*a, **h).equal?(h)  # Before and After: false
fs(*a, **h).equal?(h) # Before and After: false
```

The fact that the behavior differs when passing an empty argument splat makes it obvious that something is not working the way it is intended.  The fact that the hash is copied for C methods also makes it obvious.  Ruby 2 always copied the keyword splat hash, and I think that is the expected behavior in Ruby 3.

This bug is because of a missed check in setup_parameters_complex. If the keyword splat passed is not mutable, then it points to an existing object and not a new object, and therefore it must be copied.

I did not bisect to find the commit that introduced the problem, but this seems likely to be something I introduced during keyword argument separation.  I'm surprised that this has not been filed as a bug sooner.  Maybe it just rarely comes up in practice, and I only discovered it recently. Possibly, other developers that may have discovered this may have decided against submitting this as an issue, as we have had specs for two years that give the impression that the broken behavior was expected.

I've submitted a pull request to fix this: https://github.com/ruby/ruby/pull/8970



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:115441] [Ruby master Bug#20012] Fix keyword splat passing as regular argument

[jeremyevans0 (Jeremy Evans) via ruby-core]

Issue #20012 has been updated by jeremyevans0 (Jeremy Evans).


matz (Yukihiro Matsumoto) wrote in #note-1:
> I am in favor of this change, but at the same time concerned about compatibility. I'd like to make sure there won't be any major problems before adopting it.

Would you prefer we merge it soon and have the fix in Ruby 3.3, or should we wait until after Ruby 3.3 and put the fix in Ruby 3.4, which will allow for more testing time before release?


----------------------------------------
Bug #20012: Fix keyword splat passing as regular argument
https://bugs.ruby-lang.org/issues/20012#change-105374

* Author: jeremyevans0 (Jeremy Evans)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.3.0dev (2023-11-12 master 60e19a0b5f) [x86_64-openbsd]
* Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN
----------------------------------------
Since Ruby 3.0, Ruby has passed a keyword splat as a regular argument in the case of a call to a Ruby method where the method does not accept keyword arguments, if the method call does not contain an argument splat:

```ruby
def self.f(obj) obj end
def self.fs(*obj) obj[0] end
h = {a: 1}
f(**h).equal?(h)  # Before: true; After: false
fs(**h).equal?(h) # Before: true; After: false

a = []
f(*a, **h).equal?(h)  # Before and After: false
fs(*a, **h).equal?(h) # Before and After: false
```

The fact that the behavior differs when passing an empty argument splat makes it obvious that something is not working the way it is intended.  The fact that the hash is copied for C methods also makes it obvious.  Ruby 2 always copied the keyword splat hash, and I think that is the expected behavior in Ruby 3.

This bug is because of a missed check in setup_parameters_complex. If the keyword splat passed is not mutable, then it points to an existing object and not a new object, and therefore it must be copied.

I did not bisect to find the commit that introduced the problem, but this seems likely to be something I introduced during keyword argument separation.  I'm surprised that this has not been filed as a bug sooner.  Maybe it just rarely comes up in practice, and I only discovered it recently. Possibly, other developers that may have discovered this may have decided against submitting this as an issue, as we have had specs for two years that give the impression that the broken behavior was expected.

I've submitted a pull request to fix this: https://github.com/ruby/ruby/pull/8970



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:115638] [Ruby master Bug#20012] Fix keyword splat passing as regular argument

[mame (Yusuke Endoh) via ruby-core]

Issue #20012 has been updated by mame (Yusuke Endoh).


@jeremyevans0 In today's dev meeting, @matz said it should be fixed soon, so let's merge it. If someone complains it, we may reconsider.

----------------------------------------
Bug #20012: Fix keyword splat passing as regular argument
https://bugs.ruby-lang.org/issues/20012#change-105572

* Author: jeremyevans0 (Jeremy Evans)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.3.0dev (2023-11-12 master 60e19a0b5f) [x86_64-openbsd]
* Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN
----------------------------------------
Since Ruby 3.0, Ruby has passed a keyword splat as a regular argument in the case of a call to a Ruby method where the method does not accept keyword arguments, if the method call does not contain an argument splat:

```ruby
def self.f(obj) obj end
def self.fs(*obj) obj[0] end
h = {a: 1}
f(**h).equal?(h)  # Before: true; After: false
fs(**h).equal?(h) # Before: true; After: false

a = []
f(*a, **h).equal?(h)  # Before and After: false
fs(*a, **h).equal?(h) # Before and After: false
```

The fact that the behavior differs when passing an empty argument splat makes it obvious that something is not working the way it is intended.  The fact that the hash is copied for C methods also makes it obvious.  Ruby 2 always copied the keyword splat hash, and I think that is the expected behavior in Ruby 3.

This bug is because of a missed check in setup_parameters_complex. If the keyword splat passed is not mutable, then it points to an existing object and not a new object, and therefore it must be copied.

I did not bisect to find the commit that introduced the problem, but this seems likely to be something I introduced during keyword argument separation.  I'm surprised that this has not been filed as a bug sooner.  Maybe it just rarely comes up in practice, and I only discovered it recently. Possibly, other developers that may have discovered this may have decided against submitting this as an issue, as we have had specs for two years that give the impression that the broken behavior was expected.

I've submitted a pull request to fix this: https://github.com/ruby/ruby/pull/8970



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:115651] [Ruby master Bug#20012] Fix keyword splat passing as regular argument

[jeremyevans0 (Jeremy Evans) via ruby-core]

Issue #20012 has been updated by jeremyevans0 (Jeremy Evans).

Status changed from Open to Closed

Fixed by commit:ca204a20231f1ecabf5a7343aec49c3ea1bac90a

----------------------------------------
Bug #20012: Fix keyword splat passing as regular argument
https://bugs.ruby-lang.org/issues/20012#change-105587

* Author: jeremyevans0 (Jeremy Evans)
* Status: Closed
* Priority: Normal
* ruby -v: ruby 3.3.0dev (2023-11-12 master 60e19a0b5f) [x86_64-openbsd]
* Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN
----------------------------------------
Since Ruby 3.0, Ruby has passed a keyword splat as a regular argument in the case of a call to a Ruby method where the method does not accept keyword arguments, if the method call does not contain an argument splat:

```ruby
def self.f(obj) obj end
def self.fs(*obj) obj[0] end
h = {a: 1}
f(**h).equal?(h)  # Before: true; After: false
fs(**h).equal?(h) # Before: true; After: false

a = []
f(*a, **h).equal?(h)  # Before and After: false
fs(*a, **h).equal?(h) # Before and After: false
```

The fact that the behavior differs when passing an empty argument splat makes it obvious that something is not working the way it is intended.  The fact that the hash is copied for C methods also makes it obvious.  Ruby 2 always copied the keyword splat hash, and I think that is the expected behavior in Ruby 3.

This bug is because of a missed check in setup_parameters_complex. If the keyword splat passed is not mutable, then it points to an existing object and not a new object, and therefore it must be copied.

I did not bisect to find the commit that introduced the problem, but this seems likely to be something I introduced during keyword argument separation.  I'm surprised that this has not been filed as a bug sooner.  Maybe it just rarely comes up in practice, and I only discovered it recently. Possibly, other developers that may have discovered this may have decided against submitting this as an issue, as we have had specs for two years that give the impression that the broken behavior was expected.

I've submitted a pull request to fix this: https://github.com/ruby/ruby/pull/8970



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/