Skip to content

Fix: [for cherry-picking] Updated suspension walkers to inspect call/ne #200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
qodo-code-review wants to merge 2 commits into from
Closed

Fix: [for cherry-picking] Updated suspension walkers to inspect call/ne #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5bbb1c6
fix: 2 findings — Traverse call and new spread arguments; Traverse dynam
qodo-code-review[bot] Sep 4, 2026
0200729
fix: Link regression evidence to the G3 root gate
qodo-code-review[bot] Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion crates/bamts-compiler/src/emitter/transforms.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7055,6 +7055,7 @@ fn count_yields(expression: &Expr) -> u32 {
.iter()
.map(|argument| match argument {
CallArgument::Expression(value) => count_yields(value),
CallArgument::Spread(spread) => count_yields(&spread.argument),

@sourcery-ai sourcery-ai Bot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (broader_impact): Spread arguments are now counted as containing yields, but contains_yield still ignores CallArgument::Spread; the early eval guard therefore treats calls such as f(...(yield value)) as non-suspending and leaves the yield inside the emitted call instead of lowering it.

Triggers: When a generator contains a yield expression inside a call or constructor spread argument.

Suggested fix: Traverse CallArgument::Spread(spread).argument in contains_yield as well, keeping the suspension predicates consistent.

_ => 0,
})
.sum::<u32>()
Expand All @@ -7073,6 +7074,7 @@ fn count_yields(expression: &Expr) -> u32 {
.iter()
.map(|argument| match argument {
CallArgument::Expression(value) => count_yields(value),
CallArgument::Spread(spread) => count_yields(&spread.argument),
_ => 0,
})
.sum::<u32>()
Expand Down Expand Up @@ -7301,7 +7303,10 @@ fn machine_state_name(skip: &std::collections::HashSet<String>, temps: &[String]
}

fn call_argument_suspends(argument: &CallArgument) -> bool {
matches!(argument, CallArgument::Expression(value) if contains_yield(value))
match argument {
CallArgument::Expression(value) => contains_yield(value),
CallArgument::Spread(spread) => contains_yield(&spread.argument),
}
Comment on lines +7306 to +7309

@sourcery-ai sourcery-ai Bot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): The new match is non-exhaustive because CallArgument also has the Missing variant, as handled by rewrite_call_arguments; the crate fails to compile with a non-exhaustive-pattern error.

Suggested fix: Handle CallArgument::Missing(_) as non-suspending, or retain a wildcard arm.

Suggested change
match argument {
CallArgument::Expression(value) => contains_yield(value),
CallArgument::Spread(spread) => contains_yield(&spread.argument),
}
match argument {
CallArgument::Expression(value) => contains_yield(value),
CallArgument::Spread(spread) => contains_yield(&spread.argument),
CallArgument::Missing(_) => false,
}

}

/// Whether an assignment target subtree still holds an await, so the
Expand Down Expand Up @@ -7340,6 +7345,7 @@ fn contains_await(expression: &Expr) -> bool {
contains_await(&call.callee)
|| call.arguments.iter().any(|argument| match argument {
CallArgument::Expression(value) => contains_await(value),
CallArgument::Spread(spread) => contains_await(&spread.argument),
_ => false,
})
}
Expand All @@ -7348,6 +7354,7 @@ fn contains_await(expression: &Expr) -> bool {
contains_await(&new.callee)
|| new.arguments.iter().any(|argument| match argument {
CallArgument::Expression(value) => contains_await(value),
CallArgument::Spread(spread) => contains_await(&spread.argument),
_ => false,
})
}
Expand Down Expand Up @@ -7388,6 +7395,10 @@ fn contains_await(expression: &Expr) -> bool {
Expression::Parenthesized(inner) => contains_await(inner),
Expression::As(cast) => contains_await(&cast.expression),
Expression::NonNull(non_null) => contains_await(&non_null.expression),
Expression::Import(import) => {
contains_await(&import.source)
|| import.options.as_deref().is_some_and(contains_await)
}
_ => true,
}
}
Expand Down
4 changes: 3 additions & 1 deletion docs/solutions/logic-errors/es5-generator-suspension-walker-divergence.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,6 @@ actually mints.
- The corpus-level regression gate is the suite pair
(`cargo test -p bamts-compiler` + `cargo test -p bamts-verification` with
`BAMTS_ALLOW_NODE_COMPAT=1`), never the bare CLI `-p` path (it never
lowers the machine — refusal form only).
lowers the machine — refusal form only). These results are regression
evidence, not closure by themselves: completion requires their
receipt-backed G3 compiler root-gate linkage.