search

ruby-concurrency / concurrent-ruby

Modern concurrency tools including agents, futures, promises, thread pools, supervisors, and more. Inspired by Erlang, Clojure, Scala, Go, Java, JavaScript, and classic concurrency patterns.
https://ruby-concurrency.github.io/concurrent-ruby/
Other
5.68k stars 418 forks source link

CAtomicFixnum should probably be a private constant #981

Open eregon opened 1 year ago

eregon commented 1 year ago

Otherwise we get misleading suggestions from did-you-mean in case concurrent/atomic/atomic_fixnum is not required but the extension is loaded like:

     NameError:
       uninitialized constant Concurrent::AtomicFixnum

                 counter = AtomicFixnum.new
                           ^^^^^^^^^^^^
       Did you mean?  Concurrent::CAtomicFixnum
     # ./spec/concurrent/atomic/cyclic_barrier_spec.rb:106:in `block (4 levels) in <module:Concurrent>'

Note that even if it's private, did-you-mean is still kind of misleading:

     NameError:
       uninitialized constant Concurrent::AtomicFixnum

                   counter = AtomicFixnum.new
                             ^^^^^^^^^^^^
       Did you mean?  Concurrent::AtomicReference
     # ./spec/concurrent/atomic/cyclic_barrier_spec.rb:191:in `block (5 levels) in <module:Concurrent>'

And there are various specs checking defined? Concurrent::CAtomicFixnum and even a benchmark referencing Concurrent::CAtomicFixnum, so it's not so easy to make it private unfortunately.

Unclear if worth fixing. WIP at https://github.com/ruby-concurrency/concurrent-ruby/compare/master...eregon:concurrent-ruby:private-AtomicFixnum

eregon commented 1 year ago

Given #986, it seems important to communicate very clearly which classes are internal, e.g., using private_constant. Apparently "a class is only public if present in the documentation/README" is not clear enough.