Repository navigation
Conversation
This prevents mutations in one output template interpolation from affecting sibling output.
With the new context isolation, we don't need to explicitly introduce a new scope. We could probably get rid of `ctx.scoped` now, but there's no harm in leaving it around.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The context-isolation change introduces borrowing conflicts and alters dereference behavior, breaking previously valid interpolation expressions and borrowed inputs.
2 open issues
What changed in this PR
This PR extends Yeast’s context isolation from rules to individual output-template interpolations, preventing mutations in one interpolation from affecting its siblings.
Changes:
- Add a guard that clones and restores the user context around each interpolation.
- Add a regression test covering isolation across several interpolation forms.
- Simplify Swift translation rules by replacing explicit context scopes with isolated interpolations.
| File | Description |
|---|---|
unified/extractor/src/languages/swift/swift.rs |
Simplifies initializer and callee translation and updates context-reset documentation. |
shared/yeast/tests/test.rs |
Tests that interpolation mutations do not leak into sibling interpolations or the surrounding context. |
shared/yeast/src/build.rs |
Adds a context guard that restores the original user context when dropped. |
shared/yeast-macros/src/lower.rs |
Wraps generated interpolation code in the isolation guard. |
Review effort: Max
|
|
||
| fn isolated_interpolation(body: TokenStream) -> TokenStream { | ||
| quote! {{ | ||
| let mut ctx = ctx.cloned_user_ctx(); |
|
|
||
| fn isolated_interpolation(body: TokenStream) -> TokenStream { | ||
| quote! {{ | ||
| let mut ctx = ctx.cloned_user_ctx(); |
This branch has not been deployed
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
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.
Currently we have context isolation between rules: a change to
ctxis visible when we translate child nodes (if the translation happens after the change, of course), but these changes cannot leak to different rules.However, the same isolation does not apply to context modifications made inside of the interpolations of an output template. That is, in the following template
both
barandbazwill be evaluated in a reset context. If we swap the two field assignments, then onlybargets a reset context.This behaviour is very surprising, and likely to cause hard-to-spot bugs.
To fix this, we now clone the user context for each interpolation individually, which means they cannot interfere with each other. This is slightly less efficient, but much safer.
This also enables us to get rid of the two uses of
ctx.scopedthat we currently have, and clean up the code a bit.