ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
@ 2020-05-13 12:00 sylvain.joyeux
  2020-05-13 12:32 ` [ruby-core:98319] " shevegen
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: sylvain.joyeux @ 2020-05-13 12:00 UTC (permalink / raw
  To: ruby-core

Issue #16853 has been reported by sylvain.joyeux (Sylvain Joyeux).

----------------------------------------
Bug #16853: calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
https://bugs.ruby-lang.org/issues/16853

* Author: sylvain.joyeux (Sylvain Joyeux)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.1p83
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
The following codedef bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla "some" => "string"
~~~

**silently** outputs the following (no warnings about deprecation of keyword parameters-from-hash)

~~~
H: {}
K: {"some"=>"string"}
~~~

While 2.6.5 (and versions before it) gave

~~~
H: {"some"=>"string"}
K: {}
~~~

I would expect "the warning" that started appearing in 2.7, and **definitely** not having strings in a keyword argument hash.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ruby-core:98319] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
  2020-05-13 12:00 [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7) sylvain.joyeux
@ 2020-05-13 12:32 ` shevegen
  2020-05-13 15:39 ` [ruby-core:98322] " merch-redmine
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: shevegen @ 2020-05-13 12:32 UTC (permalink / raw
  To: ruby-core

Issue #16853 has been updated by shevegen (Robert A. Heiler).


Hmm. I am confused about the example though.

Isn't

    bla "some" => "string"

actually

  
    bla( { "some" => "string" } )

?

----------------------------------------
Bug #16853: calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
https://bugs.ruby-lang.org/issues/16853#change-85558

* Author: sylvain.joyeux (Sylvain Joyeux)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.1p83
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
The following code

~~~
def bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla "some" => "string"
~~~

**silently** outputs the following (no warnings about deprecation of keyword parameters-from-hash)

~~~
H: {}
K: {"some"=>"string"}
~~~

While 2.6.5 (and versions before it) gave

~~~
H: {"some"=>"string"}
K: {}
~~~

I would expect "the warning" that started appearing in 2.7, and **definitely** not having strings in a keyword argument hash.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ruby-core:98322] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
  2020-05-13 12:00 [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7) sylvain.joyeux
  2020-05-13 12:32 ` [ruby-core:98319] " shevegen
@ 2020-05-13 15:39 ` merch-redmine
  2020-05-13 20:18 ` [ruby-core:98334] " sylvain.joyeux
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: merch-redmine @ 2020-05-13 15:39 UTC (permalink / raw
  To: ruby-core

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

Status changed from Open to Rejected

This change was deliberate and mentioned in the 2.7.0 release announcement:

```
Non-symbols are allowed as keyword argument keys if the method accepts arbitrary keywords. [Feature #14183]

def foo(**kw); p kw; end; foo("str" => 1) #=> {"str"=>1}
```

If you want to pass a positional hash in 2.7+, you have to use a hash instead of keywords:

```ruby
def bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla({"some" => "string"})
# H: {"some"=>"string"}
# K: {}
```

----------------------------------------
Bug #16853: calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
https://bugs.ruby-lang.org/issues/16853#change-85561

* Author: sylvain.joyeux (Sylvain Joyeux)
* Status: Rejected
* Priority: Normal
* ruby -v: 2.7.1p83
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
The following code

~~~
def bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla "some" => "string"
~~~

**silently** outputs the following (no warnings about deprecation of keyword parameters-from-hash)

~~~
H: {}
K: {"some"=>"string"}
~~~

While 2.6.5 (and versions before it) gave

~~~
H: {"some"=>"string"}
K: {}
~~~

I would expect "the warning" that started appearing in 2.7, and **definitely** not having strings in a keyword argument hash.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ruby-core:98334] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
  2020-05-13 12:00 [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7) sylvain.joyeux
  2020-05-13 12:32 ` [ruby-core:98319] " shevegen
  2020-05-13 15:39 ` [ruby-core:98322] " merch-redmine
@ 2020-05-13 20:18 ` sylvain.joyeux
  2020-05-15 14:41 ` [ruby-core:98387] " mame
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: sylvain.joyeux @ 2020-05-13 20:18 UTC (permalink / raw
  To: ruby-core

Issue #16853 has been updated by sylvain.joyeux (Sylvain Joyeux).


> This change was deliberate and mentioned in the 2.7.0 release announcement:

I missed that one ... Sorry for the noise. Thanks.

----------------------------------------
Bug #16853: calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
https://bugs.ruby-lang.org/issues/16853#change-85601

* Author: sylvain.joyeux (Sylvain Joyeux)
* Status: Rejected
* Priority: Normal
* ruby -v: 2.7.1p83
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
The following code

~~~
def bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla "some" => "string"
~~~

**silently** outputs the following (no warnings about deprecation of keyword parameters-from-hash)

~~~
H: {}
K: {"some"=>"string"}
~~~

While 2.6.5 (and versions before it) gave

~~~
H: {"some"=>"string"}
K: {}
~~~

I would expect "the warning" that started appearing in 2.7, and **definitely** not having strings in a keyword argument hash.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ruby-core:98387] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
  2020-05-13 12:00 [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7) sylvain.joyeux
                   ` (2 preceding siblings ...)
  2020-05-13 20:18 ` [ruby-core:98334] " sylvain.joyeux
@ 2020-05-15 14:41 ` mame
  2020-07-02 13:55 ` [ruby-core:99029] " sylvain.joyeux
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: mame @ 2020-05-15 14:41 UTC (permalink / raw
  To: ruby-core

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


@sylvain.joyeux

Hi!  I read [your comment in the discuss RubyOnRails](https://discuss.rubyonrails.org/t/new-2-7-3-0-keyword-argument-pain-point/74980/13).  I'm sorry for bothering you about this change.  Let me explain a bit.


This change was introduced into 2.7 because the old behavior was unintentional.  In fact, Ruby 2.0.0-p0 was the same as the current (2.7) behavior.

```
$ ./bin/ruby-2.0.0-p0 -ve '
def provides(h = {}, **kw)
  p kw
end

provides("some" => "strings", as: "name")
'
ruby 2.0.0p0 (2013-02-24 revision 39474) [x86_64-linux]
{"some"=>"strings", :as=>"name"}
```

But the behavior was changed to reject non-symbol keys, and it was done without matz's confirmation.  So in a sense, this is a bug fix.

That being said, we don't change such a long-working behavior without a reason.  The reason why it was changed is because there are some methods that want to accept both symbol and non-symbol keys: `where(id: 42)` and `where("table.id" => 42)`.  This will not work in 3.0 if keyword arguments are completely separated from positional ones.  So, non-symbol keys were allowed to keyword arguments.


BTW, in that forum, you said "adding a keyword splat will break code".  This is true, even without this 2.7 change.  Please consider:

```
def m(mapping = {}); mapping; end

m(k: 1) #=> {:k=>1}
```

and if you add `**kw`,

```
def m(mapping = {}, **kw); mapping; end

m(k: 1) #=> {} # changed
```

Fixing this issue was (one of) the original motivation of 3.0 keyword arguments (#14183): consistently passing `k: 1` to keywords, and `{ k: 1 }` to positional.  This is very simple and completely solves the ambiguity.  But unfortunately, it turned out too breaking.  We decided to continue allowing the automatic conversion from keywords to positional.  I want to solve the ambiguity in future, but it is very tough.


To know your pain point precisely, I'd like to ask you a question: can you manually separate non-symbol keys?

```
def provides(h = {}, **kw)
  kw.each {|k, v| h[k] = v if k.is_a?(Symbol) }

  ...main code...
end
```

I think this code works perfectly even in 2.6.  I'm sorry for asking you to change your code, but if we change anything, anyone must pay a cost.  (No change require no cost, but I believe that it means the death of the language.)  To minimize the sum of costs, I'd like to know how pain your pain point is.

Again, I'm really sorry for bothering you, and thank you for taking time for this issue.

----------------------------------------
Bug #16853: calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
https://bugs.ruby-lang.org/issues/16853#change-85656

* Author: sylvain.joyeux (Sylvain Joyeux)
* Status: Rejected
* Priority: Normal
* ruby -v: 2.7.1p83
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
The following code

~~~
def bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla "some" => "string"
~~~

**silently** outputs the following (no warnings about deprecation of keyword parameters-from-hash)

~~~
H: {}
K: {"some"=>"string"}
~~~

While 2.6.5 (and versions before it) gave

~~~
H: {"some"=>"string"}
K: {}
~~~

I would expect "the warning" that started appearing in 2.7, and **definitely** not having strings in a keyword argument hash.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ruby-core:99029] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
  2020-05-13 12:00 [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7) sylvain.joyeux
                   ` (3 preceding siblings ...)
  2020-05-15 14:41 ` [ruby-core:98387] " mame
@ 2020-07-02 13:55 ` sylvain.joyeux
  2020-07-02 16:31 ` [ruby-core:99033] " daniel
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: sylvain.joyeux @ 2020-07-02 13:55 UTC (permalink / raw
  To: ruby-core

Issue #16853 has been updated by sylvain.joyeux (Sylvain Joyeux).


Hi Yusuke. Thanks for the long thought-out answer. I really appreciate you taking the time.

> But the behavior was changed to reject non-symbol keys, and it was done without matz's confirmation. So in a sense, this is a bug fix.

It feels like a very "ruby-core centric" perspective. This "bug" was advertised loudly with an exception for 6 years (2.1 to just-before 2.7) while it did "not exist" for one. In addition, keyword arguments started in 2.0, which means that people like me were likely to start migrating to keyword arguments later, and gradually.

Once something is "true" for such a long period, it has to be considered the established behavior. The very fact that you try to gently migrating to "pure" keyword arguments says it.

> The reason why it was changed is because there are some methods that want to accept both symbol and non-symbol keys: where(id: 42) and where("table.id" => 42).

So, because Rails. I was guessing that it would be the driver.

Now, should really the others be damned ? All the other people have to add `{}` but Rails gets a backward-incompatible core change to be able to continue doing what they do ?

Which, by the way, also hints that Rails had as of yet not migrated their `where` method to use keyword arguments ? This is what I said on the Rails Discussion thread: that next time I'll do the same and wait until the very last minute. At least, I'll be sure.

I don't mind having the change *in the long run*. But from where I stand, this looks like a backward-incompatible change that's been added to allow for forward-compatibility to please one actor in the community. One very big actor, I grant you that.

This is really bad when you are not that actor (or not using that actor's framework). What I feel is that "because rails" is breaking trust for "anybody that's not rails".  Some people are vocal that Ruby is not Rails, but this kind of decisions speak volumes. Which is natural in some way, I guess, given how big Rails is in the community.

> To know your pain point precisely, I'd like to ask you a question: can you manually separate non-symbol keys?

I am peppering my codebase with those, yes. The problem is that I also have to do that on forwarding methods to be compatible with the forward-breaking stuff in Ruby 2.6. So, has rather been a nightmare. I don't have a team of software engineers that can spend weeks on this stuff.

(I'm sorry to have taken so long to answer, but I thought I would get an email notification and I have not, I'll try and see how to make that happen)


----------------------------------------
Bug #16853: calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
https://bugs.ruby-lang.org/issues/16853#change-86404

* Author: sylvain.joyeux (Sylvain Joyeux)
* Status: Rejected
* Priority: Normal
* ruby -v: 2.7.1p83
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
The following code

~~~
def bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla "some" => "string"
~~~

**silently** outputs the following (no warnings about deprecation of keyword parameters-from-hash)

~~~
H: {}
K: {"some"=>"string"}
~~~

While 2.6.5 (and versions before it) gave

~~~
H: {"some"=>"string"}
K: {}
~~~

I would expect "the warning" that started appearing in 2.7, and **definitely** not having strings in a keyword argument hash.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ruby-core:99033] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
  2020-05-13 12:00 [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7) sylvain.joyeux
                   ` (4 preceding siblings ...)
  2020-07-02 13:55 ` [ruby-core:99029] " sylvain.joyeux
@ 2020-07-02 16:31 ` daniel
  2020-07-04 17:46 ` [ruby-core:99054] " eregontp
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: daniel @ 2020-07-02 16:31 UTC (permalink / raw
  To: ruby-core

Issue #16853 has been updated by Dan0042 (Daniel DeLorme).


> > The reason why it was changed is because there are some methods that want to accept both symbol and non-symbol keys: where(id: 42) and where("table.id" => 42).
> 
> So, because Rails. I was guessing that it would be the driver.

The idea for that change first appeared in https://bugs.ruby-lang.org/issues/14183#note-45
And the reason appears to be "because Matz thought it would be cool". I didn't really see anything about Rails in there, nor did I see anything about why you'd want to change `where("table.id" => 1)` to keyword arguments.

----------------------------------------
Bug #16853: calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
https://bugs.ruby-lang.org/issues/16853#change-86408

* Author: sylvain.joyeux (Sylvain Joyeux)
* Status: Rejected
* Priority: Normal
* ruby -v: 2.7.1p83
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
The following code

~~~
def bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla "some" => "string"
~~~

**silently** outputs the following (no warnings about deprecation of keyword parameters-from-hash)

~~~
H: {}
K: {"some"=>"string"}
~~~

While 2.6.5 (and versions before it) gave

~~~
H: {"some"=>"string"}
K: {}
~~~

I would expect "the warning" that started appearing in 2.7, and **definitely** not having strings in a keyword argument hash.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ruby-core:99054] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
  2020-05-13 12:00 [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7) sylvain.joyeux
                   ` (5 preceding siblings ...)
  2020-07-02 16:31 ` [ruby-core:99033] " daniel
@ 2020-07-04 17:46 ` eregontp
  2020-08-21 20:37 ` [ruby-core:99668] " sylvain.joyeux
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: eregontp @ 2020-07-04 17:46 UTC (permalink / raw
  To: ruby-core

Issue #16853 has been updated by Eregon (Benoit Daloze).


FWIW I don't think it was "for Rails" but rather more use cases than that.

I want to note there is also both a performance cost and a semantics complication to only allowing Symbol keys.
If there is a call like `foo(**h)`, and `h` has both Symbol and non-Symbol keys, in <2.7 the Hash is "split" which I find extremely confusing and it's also slowing things down by always having to consider "there might be an extra positional argument".
The actual semantics of it are so complicated than you probably need multiple paragraphs just to describe them.
In short, I think most people agree it's a very bad idea to "split" keyword arguments like it was done in 2.0-2.6.
Maybe raising an error for non-Symbol keys would have made sense, but I don't think that would be any more compatible.

For `provides "some" => "strings", as: "name"`
Either the user understands there are two kind of arguments and does:
`provides({"some" => "strings"}, as: "name")`
Or they just see it as a configuration Hash and then the receiving method simply extracts `as` with `h.delete(:as)`.
On the syntactic side, `provides "some" => "strings", as: "name"` has always been a single Hash, relying on the splitting was IMHO very brittle, and it seems easy to workaround with just `def provides(h)`.

----------------------------------------
Bug #16853: calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
https://bugs.ruby-lang.org/issues/16853#change-86428

* Author: sylvain.joyeux (Sylvain Joyeux)
* Status: Rejected
* Priority: Normal
* ruby -v: 2.7.1p83
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
The following code

~~~
def bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla "some" => "string"
~~~

**silently** outputs the following (no warnings about deprecation of keyword parameters-from-hash)

~~~
H: {}
K: {"some"=>"string"}
~~~

While 2.6.5 (and versions before it) gave

~~~
H: {"some"=>"string"}
K: {}
~~~

I would expect "the warning" that started appearing in 2.7, and **definitely** not having strings in a keyword argument hash.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ruby-core:99668] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
  2020-05-13 12:00 [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7) sylvain.joyeux
                   ` (6 preceding siblings ...)
  2020-07-04 17:46 ` [ruby-core:99054] " eregontp
@ 2020-08-21 20:37 ` sylvain.joyeux
  2020-08-22  9:56 ` [ruby-core:99671] " eregontp
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: sylvain.joyeux @ 2020-08-21 20:37 UTC (permalink / raw
  To: ruby-core

Issue #16853 has been updated by sylvain.joyeux (Sylvain Joyeux).


> If there is a call like foo(**h), and h has both Symbol and non-Symbol keys, in <2.7 the Hash is "split" which I find extremely confusing and it's also slowing things down by always having to consider "there might be an extra positional argument".

Keyword splat would not allow anything but symbols as keys pre-2.7.

~~~
irb(main):001:0> h = { k: 2, "c" => 20 }
=> {:k=>2, "c"=>20}
irb(main):002:0> def m(**kw)
irb(main):003:1> end
=> :m
irb(main):004:0> m(**h)
Traceback (most recent call last):
        4: from /home/doudou/.rbenv/versions/2.6.5/bin/irb:23:in `<main>'
        3: from /home/doudou/.rbenv/versions/2.6.5/bin/irb:23:in `load'
        2: from /home/doudou/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/irb-1.0.0/exe/irb:11:in `<top (required)>'
        1: from (irb):4
TypeError (hash key "c" is not a Symbol)
irb(main):005:0> 
~~~

I do want to apologize for my "for Rails" comment. This was unfair to the core team's work.

When it comes to optimization, though, I would assume that the keywords are all symbols would open a lot more doors.


----------------------------------------
Bug #16853: calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
https://bugs.ruby-lang.org/issues/16853#change-87154

* Author: sylvain.joyeux (Sylvain Joyeux)
* Status: Rejected
* Priority: Normal
* ruby -v: 2.7.1p83
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
The following code

~~~
def bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla "some" => "string"
~~~

**silently** outputs the following (no warnings about deprecation of keyword parameters-from-hash)

~~~
H: {}
K: {"some"=>"string"}
~~~

While 2.6.5 (and versions before it) gave

~~~
H: {"some"=>"string"}
K: {}
~~~

I would expect "the warning" that started appearing in 2.7, and **definitely** not having strings in a keyword argument hash.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ruby-core:99671] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
  2020-05-13 12:00 [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7) sylvain.joyeux
                   ` (7 preceding siblings ...)
  2020-08-21 20:37 ` [ruby-core:99668] " sylvain.joyeux
@ 2020-08-22  9:56 ` eregontp
  2020-08-22  9:59 ` [ruby-core:99672] " eregontp
  2020-08-23  3:14 ` [ruby-core:99673] " daniel
  10 siblings, 0 replies; 12+ messages in thread
From: eregontp @ 2020-08-22  9:56 UTC (permalink / raw
  To: ruby-core

Issue #16853 has been updated by Eregon (Benoit Daloze).


sylvain.joyeux (Sylvain Joyeux) wrote in #note-9:
> Keyword splat would not allow anything but symbols as keys pre-2.7.

Indeed, so what happens is the Hash is magically split in two if it has non-Symbol keys (or `raise`s with `**`):

```
Ruby 2.6.6

$ ruby -e 'def m(*args, **kwargs); [args, kwargs]; end; h = { k: 2, "c" => 20 }; p m(h)'  
[[{"c"=>20}], {:k=>2}]

$ ruby -e 'def m(*args, **kwargs); [args, kwargs]; end; h = { k: 2, "c" => 20 }; p m(**h)'
Traceback (most recent call last):
-e:1:in `<main>': hash key "c" is not a Symbol (TypeError)
```

> When it comes to optimization, though, I would assume that the keywords are all symbols would open a lot more doors.

No, it just added more checks as every call with keyword arguments would need to check all keys are Symbols.

----------------------------------------
Bug #16853: calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
https://bugs.ruby-lang.org/issues/16853#change-87158

* Author: sylvain.joyeux (Sylvain Joyeux)
* Status: Rejected
* Priority: Normal
* ruby -v: 2.7.1p83
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
The following code

~~~
def bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla "some" => "string"
~~~

**silently** outputs the following (no warnings about deprecation of keyword parameters-from-hash)

~~~
H: {}
K: {"some"=>"string"}
~~~

While 2.6.5 (and versions before it) gave

~~~
H: {"some"=>"string"}
K: {}
~~~

I would expect "the warning" that started appearing in 2.7, and **definitely** not having strings in a keyword argument hash.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ruby-core:99672] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
  2020-05-13 12:00 [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7) sylvain.joyeux
                   ` (8 preceding siblings ...)
  2020-08-22  9:56 ` [ruby-core:99671] " eregontp
@ 2020-08-22  9:59 ` eregontp
  2020-08-23  3:14 ` [ruby-core:99673] " daniel
  10 siblings, 0 replies; 12+ messages in thread
From: eregontp @ 2020-08-22  9:59 UTC (permalink / raw
  To: ruby-core

Issue #16853 has been updated by Eregon (Benoit Daloze).


FWIW I also find the `where("table.id" => 42)` example unconvincing.
That could work just fine with `def where(conditions)`, no need and it seems no much gains for keyword arguments there.

----------------------------------------
Bug #16853: calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
https://bugs.ruby-lang.org/issues/16853#change-87159

* Author: sylvain.joyeux (Sylvain Joyeux)
* Status: Rejected
* Priority: Normal
* ruby -v: 2.7.1p83
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
The following code

~~~
def bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla "some" => "string"
~~~

**silently** outputs the following (no warnings about deprecation of keyword parameters-from-hash)

~~~
H: {}
K: {"some"=>"string"}
~~~

While 2.6.5 (and versions before it) gave

~~~
H: {"some"=>"string"}
K: {}
~~~

I would expect "the warning" that started appearing in 2.7, and **definitely** not having strings in a keyword argument hash.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [ruby-core:99673] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
  2020-05-13 12:00 [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7) sylvain.joyeux
                   ` (9 preceding siblings ...)
  2020-08-22  9:59 ` [ruby-core:99672] " eregontp
@ 2020-08-23  3:14 ` daniel
  10 siblings, 0 replies; 12+ messages in thread
From: daniel @ 2020-08-23  3:14 UTC (permalink / raw
  To: ruby-core

Issue #16853 has been updated by Dan0042 (Daniel DeLorme).


> > When it comes to optimization, though, I would assume that the keywords are all symbols would open a lot more doors.
>
> No, it just added more checks as every call with keyword arguments would need to check all keys are Symbols.

If a Hash contains only Symbol keys I can easily imagine an optimized implementation that takes advantage of Symbol specifics to be more efficient that the general implementation. This would translate to increased efficiency for every method call that uses keyword arguments. Then double-splatting a hash could be optimized by just checking if the hash uses the Symbol-only implementation. Really, I think the current approach closes the door on a lot of potential optimizations.

> FWIW I also find the `where("table.id" => 42)` example unconvincing.
> That could work just fine with `def where(conditions)`, no need and it seems no much gains for keyword arguments there.

Exactly, AFAIU one of the of goals of "real" keyword arguments was to make a clearer distinction between "keywords" and "just a hash". And to me keywords really mean named arguments, in other words they're supposed to resolve to a local variable in a method. Since `"table.id"` can never be a local variable it should not be a keyword argument; it's the definition of a "data" hash. Now it feels like keyword arguments have just become a different way to pass around hash data; the boundary between keyword and hash has become _more_ blurred.

----------------------------------------
Bug #16853: calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7)
https://bugs.ruby-lang.org/issues/16853#change-87160

* Author: sylvain.joyeux (Sylvain Joyeux)
* Status: Rejected
* Priority: Normal
* ruby -v: 2.7.1p83
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
The following code

~~~
def bla(hash = {}, **kw)
    puts "H: #{hash}"
    puts "K: #{kw}"
end

bla "some" => "string"
~~~

**silently** outputs the following (no warnings about deprecation of keyword parameters-from-hash)

~~~
H: {}
K: {"some"=>"string"}
~~~

While 2.6.5 (and versions before it) gave

~~~
H: {"some"=>"string"}
K: {}
~~~

I would expect "the warning" that started appearing in 2.7, and **definitely** not having strings in a keyword argument hash.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-08-23  3:14 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-13 12:00 [ruby-core:98318] [Ruby master Bug#16853] calling bla(hash, **kw) with a string-based hash passes the strings into **kw (worked < 2.7) sylvain.joyeux
2020-05-13 12:32 ` [ruby-core:98319] " shevegen
2020-05-13 15:39 ` [ruby-core:98322] " merch-redmine
2020-05-13 20:18 ` [ruby-core:98334] " sylvain.joyeux
2020-05-15 14:41 ` [ruby-core:98387] " mame
2020-07-02 13:55 ` [ruby-core:99029] " sylvain.joyeux
2020-07-02 16:31 ` [ruby-core:99033] " daniel
2020-07-04 17:46 ` [ruby-core:99054] " eregontp
2020-08-21 20:37 ` [ruby-core:99668] " sylvain.joyeux
2020-08-22  9:56 ` [ruby-core:99671] " eregontp
2020-08-22  9:59 ` [ruby-core:99672] " eregontp
2020-08-23  3:14 ` [ruby-core:99673] " daniel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).