Verify to_s is string or fall back on anyToString#8940
Conversation
|
There's no spec specifically for this case, but without the patch it fails a hand-written example designed to lazily JIT: The closest relevant spec is here, but it only ensures the result is a string: jruby/spec/ruby/language/string_spec.rb Lines 136 to 147 in 5465a89 I will add a more specific spec that ensures non-string to_s results are not included in the dynamic string. |
|
@eregon The existing spec for this hardly verified anything, so I replaced it with one that confirms a non-String to_s result is discarded when building a dynamic string. Let me know if you have thoughts. |
|
@eregon A thought: perhaps better to |
|
The changes to spec/ruby/language/string_spec.rb look fine. I agree, it's clearer and safer to test that it looks like a usual inspect rather than check something is not there (which would pass e.g. if empty). |
Logic for "AsString" in a dynamic string must verify the result of calling to_s is a String, or else use anyToString on the original object. Previous logic here only called to_s and did not verify the result, proceeding to append non-string values like Integer or Float rather than falling back. The patch here adds an additional guard around the to_s call to perform this check and fallback. This only affects the rare case where someone has implemented a to_s method that does not return a String, and is only observable once the dstring has been optimized and invoked twice.
The old spec only verified that a dynamic string would be successfully created when an interpolated object returned a non-string value from to_s. This enhances the spec to also verify that the non-string value will be discarded in favor of the generic "hashy" inspect string.
headius
force-pushed
the
as_string_type_guarding
branch
from
74a1904 to
341b9f4
Compare
2 participants
Logic for "AsString" in a dynamic string must verify the result of calling to_s is a String, or else use anyToString on the original object. Previous logic here only called to_s and did not verify the result, proceeding to append non-string values like Integer or Float rather than falling back. The patch here adds an additional guard around the to_s call to perform this check and fallback.
This only affects the rare case where someone has implemented a to_s method that does not return a String, and is only observable once the dstring has been optimized and invoked twice.
As this is a bit invasive, I'm marking it for 10.0.3.0 so we can create more specs and do more verification that it works correctly before release.