hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-07-11 22:45:33 +02:00
Code Issues Packages Projects Releases Wiki Activity

yeast: Add BuildCtx::scoped for isolated context modification

Browse Source 
A previous commit added a translate_reset method on BuildCtx, which had
the effect of performing a translation in a completely empty context.

One issue with this is that this is an all-or-nothing proposition -- If
you want to preserve _some_ parts of the context, you have to do
something more complicated. Moreover, if you introduce a contextual
value that _should_ be preserved, all of the existing uses of
translate_reset now silently do the wrong thing.

There are two patterns that we want to address. The first one is "modify
the context in some way, then do a translation". If the translation is
the last step of a Rust block, then we don't actually need
translate_reset -- we could just reset the context and then call
`translate`. The fact that the outer context is restored afterwards
means it's okay to make destructive changes to `ctx.user_ctx` -- none of
these changes will persist.

The second pattern is the same, but where we want to do more
translations using the original context, after having performed a
translation with a modified context. In this case, we cannot just
overwrite the context, since that would invalidate the subsequent
translations.

Instead, we introduce a new method `ctx.scoped` which takes a closure as
an argument. With this we can now write

```
ctx.scoped(|ctx| ctx.reset(); ctx.translate(...));
```

and the closure is run with a copy of `ctx` that has a clone of
`user_ctx` on the inside, so no changes will persist.

(You may wonder: why not just clone `ctx` and use the clone? The answer
is that `ctx` owns mutable pointers to the AST etc., and this makes it
awkward to just "clone" it. The closure circumvents this issue nicely,
since it can borrow these pointers internally.)

For now, this rewrite has the same behaviour as the version that used
translate_reset -- we clear the entire `user_ctx`. However, we could
imagine being more fine-grained in this approach, by implementing, say,
SwiftContext::reset_modifiers (which would only affect what modifiers
are currently in the context, leaving everything else as-is).
This commit is contained in:
Taus
2026-07-04 15:00:48 +00:00
parent 8272848620
commit 13510b1ddd
3 changed files with 121 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

96
shared/yeast/src/build.rs
View File

@@ -161,52 +161,74 @@ impl<'a, C> BuildCtx<'a, C> {
}
impl<C: Clone> BuildCtx<'_, C> {
/// Recursively translate a node via the framework's rule machinery.
/// In a OneShot phase, applies OneShot rules to the given node and
/// returns the resulting node ids. In a Repeating phase, errors
/// (translation is not meaningful when input and output share a
/// schema).
/// Recursively translate every id in the given iterable via the
/// framework's rule machinery. In a OneShot phase, applies OneShot
/// rules to each id and returns the accumulated resulting node ids
/// in order. In a Repeating phase, errors (translation is not
/// meaningful when input and output share a schema).
///
/// The single-`Id` case works too, because `Id: IntoIterator<Item
/// = Id>` as a singleton iterator — so `ctx.translate(some_id)?`
/// returns a `Vec<Id>` containing whatever `some_id` translated to.
///
/// Errors if this `BuildCtx` was constructed by hand (without a
/// translator handle) — for example, in unit tests that don't go
/// through the rule driver.
pub fn translate<I: Into<Id>>(&mut self, id: I) -> Result<Vec<Id>, String> {
let id = id.into();
match &self.translator {
Some(t) => t.translate(self.ast, self.user_ctx, id),
None => Err("translate() called on a BuildCtx without a translator handle".into()),
}
}
/// Translate every node in an iterator with a **fresh** user context
/// (reset to `C::default()`), restoring the previous context afterwards.
///
/// Use when descending into a subtree — a body, expression, or statement
/// list — that must not inherit any of the surrounding translation
/// context (for example an enclosing binding modifier). Accepts optional
/// (`Option<Id>`) and repeated (`Vec<Id>`) captures (both `IntoIterator`);
/// for a single `Id`, wrap it in `std::iter::once(id)`.
pub fn translate_reset<I: Into<Id>>(
pub fn translate<I: Into<Id>>(
&mut self,
ids: impl IntoIterator<Item = I>,
) -> Result<Vec<Id>, String>
where
C: Default,
{
let saved = std::mem::take(&mut *self.user_ctx);
) -> Result<Vec<Id>, String> {
let translator = self
.translator
.as_ref()
.ok_or("translate() called on a BuildCtx without a translator handle")?;
let mut out = Vec::new();
let mut result = Ok(());
for id in ids {
match self.translate(id) {
Ok(v) => out.extend(v),
Err(e) => {
result = Err(e);
break;
}
}
let translated = translator.translate(self.ast, self.user_ctx, id.into())?;
out.extend(translated);
}
*self.user_ctx = saved;
result.map(|()| out)
Ok(out)
}
/// Run `f` with a temporary child [`BuildCtx`] whose `user_ctx` is
/// a fresh clone of the current one, sharing everything else
/// (`ast`, `captures`, `fresh`, `source_range`, `translator`) by
/// re-borrow. Any mutations `f` makes to the child's `user_ctx`
/// are discarded when it returns — no restore needed, because the
/// mutations only ever happened on a local clone.
///
/// Use for the rare rule that needs to translate a subtree under a
/// modified context *and then continue using its own (unmodified)
/// context afterwards*. For rules where the modified translation
/// is the last use of `ctx`, mutate `ctx` in place — the
/// framework's rule-boundary save/restore cleans up on rule exit.
///
/// Example: an outer rule that translates one child subtree with a
/// reset context, then continues with the outer context intact:
///
/// ```ignore
/// let val = ctx.scoped(|ctx| {
/// ctx.reset();
/// ctx.translate(val)
/// })?;
/// // `ctx` here is untouched by the reset inside the closure.
/// let other = ctx.translate(other_id)?;
/// ```
pub fn scoped<F, R>(&mut self, f: F) -> R
where
F: for<'b> FnOnce(&mut BuildCtx<'b, C>) -> R,
{
let mut child_user_ctx = self.user_ctx.clone();
let mut child = BuildCtx {
ast: &mut *self.ast,
captures: self.captures,
fresh: self.fresh,
source_range: self.source_range,
user_ctx: &mut child_user_ctx,
translator: self.translator,
};
f(&mut child)
// child_user_ctx dropped; the outer `self` is unaffected.
}
}

20
shared/yeast/src/lib.rs
View File

@@ -741,6 +741,16 @@ pub struct TranslatorHandle<'a, C> {
inner: TranslatorImpl<'a, C>,
}
// Manual `Copy` / `Clone` so `TranslatorHandle<'_, C>: Copy` holds
// regardless of whether `C: Copy`. `TranslatorImpl` contains only
// shared references, which are `Copy` unconditionally.
impl<C> Copy for TranslatorHandle<'_, C> {}
impl<C> Clone for TranslatorHandle<'_, C> {
fn clone(&self) -> Self {
*self
}
}
/// Internal phase-specific translation state. Kept private — callers
/// interact with [`TranslatorHandle`] only.
enum TranslatorImpl<'a, C> {
@@ -761,6 +771,16 @@ enum TranslatorImpl<'a, C> {
Repeating,
}
// Manual `Copy` / `Clone` so `TranslatorImpl<'_, C>: Copy` holds
// regardless of whether `C: Copy`. All variants hold only shared
// references and small `Copy` scalars.
impl<C> Copy for TranslatorImpl<'_, C> {}
impl<C> Clone for TranslatorImpl<'_, C> {
fn clone(&self) -> Self {
*self
}
}
impl<'a, C: Clone> TranslatorHandle<'a, C> {
/// Recursively apply OneShot rules to `id` and return the resulting
/// node ids. Errors in a Repeating phase (where translation is not

59
unified/extractor/src/languages/swift/swift.rs
View File

@@ -45,12 +45,29 @@ impl SwiftContext {
///
/// True exactly when an enclosing binding has published its modifier into
/// `outer_modifiers`. This is reliable because non-binding subtrees
/// (bodies, initializer values, ...) are translated with a reset context
/// (see `BuildCtx::translate_reset`), so a bare identifier only sees a
/// (bodies, initializer values, ...) are translated after resetting the
/// context (see `reset`), so a bare identifier only sees a
/// non-empty `outer_modifiers` when it really is a binding.
fn in_binding_pattern(&self) -> bool {
!self.outer_modifiers.is_empty()
}
/// Clear the context fields that must not propagate into an
/// expression / statement / body subtree.
///
/// Mirrors `Default::default()` for `SwiftContext` today, but is a
/// named method so future context fields can opt in or out of
/// clearing here per-field.
///
/// Called before recursively translating a body / initializer
/// slot. Most rules mutate `ctx` in place — the framework's
/// rule-boundary snapshot/restore cleans up on exit. Rules that
/// need the outer context intact *after* the reset-and-translate
/// (see e.g. the `property_binding` willSet/didSet rule) wrap the
/// mutation in `ctx.scoped(...)` instead.
fn reset(&mut self) {
*self = SwiftContext::default();
}
}
/// Build a freshly-created `chained_declaration` modifier node if
@@ -239,7 +256,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
name: (identifier #{name})
type: {ty}
accessor_kind: (accessor_kind "get")
body: (block stmt: {ctx.translate_reset(body)?}))
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
),
// Stored property with willSet/didSet observers (initializer
// optional) → a `variable_declaration` followed by one
@@ -260,12 +277,20 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
observers: (willset_didset_block willset: _? @@ws didset: _? @@ds))
=>
{{
// The initializer value must not inherit the binding context
// (it may contain patterns, e.g. a switch expression), so
// translate it with a reset context. The observers keep the
// context: each willSet/didSet accessor emits the binding
// modifier and resets its own body.
let val = ctx.translate_reset(val)?;
// The initializer value must not inherit the binding
// context (it may contain patterns, e.g. a switch
// expression), so translate it inside a `ctx.scoped`
// block — the block receives a temporary `ctx` whose
// `user_ctx` is a clone; mutations to it are discarded
// when the block returns, so the outer `ctx` is intact
// for the observer loop below. The observers keep the
// outer context: each willSet/didSet accessor emits
// the binding modifier and, in turn, resets the
// context for its own body.
let val = ctx.scoped(|ctx| {
ctx.reset();
ctx.translate(val)
})?;
let var_decl = tree!(
(variable_declaration
@@ -295,7 +320,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
// The enclosing `property_declaration` leads `ctx.outer_modifiers`
// with the `let`/`var` binding modifier, so the auto-translated name
// pattern (the LHS) becomes a binding, while the initializer value is
// translated with a reset context (see `translate_reset`).
// translated with a reset context (see `SwiftContext::reset`).
rule!(
(property_binding
name: @pattern
@@ -307,7 +332,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
modifier: {chained_modifier(&mut ctx)}
pattern: {pattern}
type: {ty}
value: {ctx.translate_reset(val)?}) // reset context: the initializer must not see the binding
value: {ctx.reset(); ctx.translate(val)?})
),
// property_declaration: flatten declarators (each may translate
// to multiple nodes — variable_declaration and/or
@@ -1118,7 +1143,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
name: {ctx.property_name.ok_or("computed_getter outside property_binding context")?}
type: {ctx.property_type}
accessor_kind: (accessor_kind "get")
body: (block stmt: {ctx.translate_reset(body)?}))
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
),
// Computed setter with explicit parameter name.
rule!(
@@ -1131,7 +1156,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
type: {ctx.property_type}
accessor_kind: (accessor_kind "set")
parameter: (parameter pattern: (name_pattern identifier: (identifier #{param})))
body: (block stmt: {ctx.translate_reset(body)?}))
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
),
// Computed setter without explicit parameter name; body optional.
rule!(
@@ -1143,7 +1168,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
name: {ctx.property_name.ok_or("computed_setter outside property_binding context")?}
type: {ctx.property_type}
accessor_kind: (accessor_kind "set")
body: (block stmt: {ctx.translate_reset(body)?}))
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
),
// Computed modify → accessor_declaration
rule!(
@@ -1155,7 +1180,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
name: {ctx.property_name.ok_or("computed_modify outside property_binding context")?}
type: {ctx.property_type}
accessor_kind: (accessor_kind "modify")
body: (block stmt: {ctx.translate_reset(body)?}))
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
),
// willset/didset block — spread to children (only reachable as a
// fallback; the outer property_binding manual rule normally
@@ -1173,7 +1198,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
modifier: {chained_modifier(&mut ctx)}
name: {ctx.property_name.ok_or("willset_clause outside property_binding context")?}
accessor_kind: (accessor_kind "willSet")
body: (block stmt: {ctx.translate_reset(body)?}))
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
),
// didset clause → accessor_declaration (body optional).
rule!(
@@ -1184,7 +1209,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
modifier: {chained_modifier(&mut ctx)}
name: {ctx.property_name.ok_or("didset_clause outside property_binding context")?}
accessor_kind: (accessor_kind "didSet")
body: (block stmt: {ctx.translate_reset(body)?}))
body: (block stmt: {ctx.reset(); ctx.translate(body)?}))
),
// Preprocessor conditionals — unsupported
rule!((diagnostic) => (unsupported_node)),