-
Notifications
You must be signed in to change notification settings - Fork 271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Explicitly freeze concatenated-String
constants to unbreak on non-main Ractor
s.
#449
Explicitly freeze concatenated-String
constants to unbreak on non-main Ractor
s.
#449
Conversation
Well, it also needs to work. Did you not try out your change? :) irb(main):001:0> FOO = "this is foo"
=> "this is foo"
irb(main):002:0> %q{FOO}
=> "FOO"
irb(main):003:0> %q{#{FOO}}
=> "\#{FOO}" I guess you wanted
but I think |
37d2285
to
0f5c03d
Compare
Sure, that syntax is fine. I just want this gem to stop blowing up my entire app :) |
Can you add a test too? |
0f5c03d
to
6390312
Compare
Can you resolve the conflict? |
6390312
to
178bcd9
Compare
When building
String
s with theString#concat
/String#+
methods, the receiver and argumentString
s are literals, but the resulting value is not, so they are left unfrozen when we rely only on thefrozen_string_literal
directive.Switch to the
%q
non-interpolableString
literal syntax for these because it feels less gross to me than e.g.HOST = (UNRESERVED + SUB_DELIMS + "\\[\\:\\]").freeze
. This syntax was present in Ruby 2.0, which is Addressable's minimum requirement :)Before
Concatenated
String
s are not frozen when thefrozen_string_literal: true
directive is used.…and we encounter a
Ractor::IsolationError
when trying to useAddressable::URI::parse
in a non-mainRactor
:After
String
s are frozen, and Addressable can be used in a non-mainRactor
:Even though
Ractor::shareable?
still returnsfalse
here, the Ractor "shareable objects" docs say frozenString
objects are shareable. We could additionally callRactor::make_shareable
, but there seems to be no reason to since it does work: