[ruby-core:122692] [Ruby Bug#21396] Set#initialize should call Set#add on items passed in

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


knu (Akinori MUSHA) wrote in #note-8:
> Considering the feedback we've received about compatibility in the new experimental Set implementation, it may be in our best interest to revert to the pure-Ruby version.
> 
> If improving performance and reducing memory footprint remain crucial, one option would be to keep only the underlying data structure and a few core methods from set.c, while leaving the rest written in pure Ruby.

I don't think reverting to the pure Ruby version is in our best interests.  I do understand the backwards compatibility issues, though to be honest, all issues raised so far are cases where users were relying on implementation details (of the form method `a` calls method `b`).  I think the benefits of making Set similar to Array and Hash exceed the costs of the backwards compatibility issues.  It's not just performance and reduced memory usage, but also thread safety.

Consider `add?`

```ruby
  def add?(o)
    add(o) unless include?(o)
  end
```

Keeping backwards compatibility (`add?` calls `include?` and possibly `add`) here requires giving up thread safety.  With the pure Ruby implementation, `add?` can return a value that was already added to the set, if another thread concurrently added it to the set between the `include?` and `add` calls.  This type of thread safety issue can perhaps be accepted from a standard library, but should not be accepted for a core class. Since Ruby 3.2, Set has been closer to core class than standard library, due to the autoload.

As you mentioned, restoring backwards compatibility is possible while keeping the same optimized data structure. It's mostly a question of how far we want to go. If we want to support backwards compatibility for Set subclasses and method overrides, I think we should assess each situation on a case-by-case basis. I can implement the changes to have `Set#initialize` call `Set#add` (this issue) and have `Set.[]` call `Set#initialize` (#21375), if that is the behavior you prefer. We can officially support such method overriding by adding tests for the behavior, so users who override the methods are not relying on implementation details.

----------------------------------------
Bug #21396: Set#initialize should call Set#add on items passed in
https://bugs.ruby-lang.org/issues/21396#change-113970

* 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:

```
> ruby -v test.rb
ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [arm64-darwin24]
#<Foo: {3}>
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/
______________________________________________
 ruby-core mailing list -- [e-mail]
 To unsubscribe send an email to [e-mail]
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/