Explicit preparation phase without STATE #333
Closed
Conversation
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
Since git 2.23 .gitignore that is a symbolic link is ignored. I made them explicit now (though I'm not sure they're even needed)
So simplify the return value.
Our goal is to get pure functions that don't affect STATE.
It turns out the dropping and relocking state in the right order is essential for the test suite to even complete...
Note that prepare isn't pure here as it mutates a class. We may want to rearrange things later.
…t's state related. It's still going to need revision later as it depends on state modification to work. The class should only be modified once it is explicitly registered.
…stream generation.
Xenira
requested changes
Oct 22, 2024
There was a problem hiding this comment.
Can you check if this mr builds on top of current master. A little confused seeing these changes again.
|
Makes sense! I am going to take a look soon! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
State (through the static
STATE) is modified and used throughout ext-php-rs, but it's giving rise to various problems during development - the order in which the run compiler runs things is not guaranteed, and it appears that at least in some cases (in rust analyzer?) the system gets confused. See #327.To make this go away, I want to introduce an explicit registration mode. This is a preparatory refactor (which should introduce no behavioral changes) to help go into this direction, unweaving STATE access and modification from generation of token streams. It doesn't do anything by itself, but hopefully clears the ground.
The next step would be to see whether we can defer building and registration to a later phase. In particular, the
ClassBuilderbuildmethod registers as a side-effect, though the other builders don't appear to do so.This is a step in the direction of #329