Revisit chilled strings and pass all specs#8375
Merged
headius merged 7 commits intojruby:9.5-devfrom Oct 14, 2024
Merged
Conversation
We have too many ways to check for frozen status and error etc, and they all need to be chilled-aware, so for now just override them in RubyString. In the future we'll want to sort this out to one master method that subclasses can override for specialized frozen checks.
ObjectSpace for string is already problematic, but in this case we have two specialized types of strings that are born frozen before they are fully initialized. Adding these into ObjectSpace requires installing a hidden variable, which triggers frozen checks, which fire causing a FrozenError to try to inspect an incompletely- initialized object. It's simpler to just say that FString (and the Debug version) never go into ObjectSpace.
ObjectSpace wants to be able to assign a hidden variable, but calling checkFrozen here leads to child class implementations being called that may have other side effects. This is another place where the frozen checks need to be unified and cleaned up, but for now I restore the !isFrozen short circuit and revert the checkFrozen call to avoid this unintended behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixing the one failure (chilled strings should not be
frozen?) led to some refactoring elsewhere in order to keep specs passing. All are green now.