Issue #21396 has been updated by jeremyevans0 (Jeremy Evans). Eregon (Benoit Daloze) wrote in #note-13:
jeremyevans0 (Jeremy Evans) wrote in #note-11:
knu (Akinori MUSHA) wrote in #note-10:
Jeremy, thanks for the reply.
Your point about thread-safety is well taken. It is an important advantage. As a possible compromise, we could keep the C backing and switch the behavior of methods when the class is subclassed in exchange for losing thread-safety. It's just an idea and not the cleanest solution, though.
I think that is a reasonable compromise. It keeps the advantages of core Set when not subclassed, and keeps backwards compatibility for subclasses (at least, subclasses that don't access `@hash`). Please let me know if you would like me to work on that.
Yes, this sounds like a good way, it avoids breaking code and keeps performance optimal for Set. The only downside is some extra complexity but only some conditions in a couple methods, no big deal. And the thread-safety aspect is pretty minor as explained in my other comment (and obviously it wasn't guaranteed before so that's not breaking anything).
ruby -v test.rb ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [arm64-darwin24] #<Foo: {3}>
I submitted a pull request that implements what @knu suggested: https://github.com/ruby/ruby/pull/15228 It allows subclasses to use the new core implementation and avoid the backwards compatible layer if they subclass from `Set::CoreSet`. ---------------------------------------- Bug #21396: Set#initialize should call Set#add on items passed in https://bugs.ruby-lang.org/issues/21396#change-115239 * Author: tenderlovemaking (Aaron Patterson) * Status: Open * Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- ```ruby class Foo < Set def add(item) = super(item.bytesize) end x = Foo.new(["foo"]) p x p x.include?(3) ``` On Ruby 3.4 the output is this: ``` true ``` On Ruby master the output is this: ```
make run ./miniruby -I./lib -I. -I.ext/common -r./arm64-darwin24-fake ./test.rb #<Set: {"foo"}> false
The bug is that `initialize` is not calling `add` for the elements passed in, so the subclass doesn't get a chance to change them.
I've sent a PR here: https://github.com/ruby/ruby/pull/13518
--
https://bugs.ruby-lang.org/