Let a ghost term name generic- and Self-typed variables - #232
Conversation
149a0ea to
86fd516
Compare
86fd516 to
f981e67
Compare
f981e67 to
0878392
Compare
`invariant!` builds its `#[thrust::formula_fn]` from a closure and the context the closure was written in: the in-scope generics are re-declared on the function and instantiated via turbofish, `Self` becomes the impl's self type or a synthetic type parameter in a trait, and the receiver `self` becomes a `__thrust_self` parameter. None of that is particular to an invariant -- it is what any formula written inside a function body needs to survive being lifted out of it. Move it to `formula_fn_lifting`, which takes the parameters and the body and returns the item plus the expression naming it, and leave `invariant` with the part that is its own: reading the closure and emitting the marker call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PJ6XNNsSBdPkAzrWHftvqV
A formula lifted out of a function body becomes a free function, where `self` is not a legal parameter name, so one naming the receiver arrives under a synthetic name that stands for the value debug info records as `self`. The loop-invariant path knew that rule inline; the ghost path did not know it at all, and looked up the synthetic name as if a variable of that name were live. Name the rule and have both paths read the parameter through it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PJ6XNNsSBdPkAzrWHftvqV
A ghost term is lifted into a free `#[thrust::formula_fn]`, which inherits neither the
enclosing function's generics nor `Self`, so a term could only name variables of
concrete type. `#[thrust_macros::context]` already threads that context into
`invariant!`; thread it into `ghost!` too, through the same lifting:
#[thrust_macros::context]
impl Counter {
fn record(&mut self, x: i64) {
self.count += 1;
self.seen = thrust_macros::ghost!(
|self: &mut Self, x: i64| -> Seq<Int> { (*self).1.push(x) }
);
}
}
The introduced value is parameter `0` of the lifted function, so it passes through as
an ordinary parameter: a value type naming `Self` or a generic is rewritten along with
the rest, while the `__ghost_marker::<_, T>` turbofish keeps the type as written, the
marker call being in the host's own scope.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PJ6XNNsSBdPkAzrWHftvqV
0878392 to
2d34fe7
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Macro matching can incorrectly rewrite unrelated macros named ghost.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds context-aware lifting so ghost! terms can reference generic and Self-typed variables.
Changes:
- Extracts shared formula-function lifting from
invariant!. - Threads enclosing context into
ghost!. - Adds analyzer support and paired UI tests.
File summaries
| File | Description |
|---|---|
thrust-macros/src/lib.rs |
Registers lifting module and context-aware ghost macro. |
thrust-macros/src/invariant.rs |
Uses shared lifting logic. |
thrust-macros/src/ghost.rs |
Adds context-aware ghost expansion. |
thrust-macros/src/formula_fn_lifting.rs |
Implements shared formula lifting. |
thrust-macros/src/context.rs |
Injects context into ghost macros. |
src/analyze/annot_fn.rs |
Centralizes lifted parameter name resolution. |
src/analyze/local_def.rs |
Uses shared name resolution for invariants. |
src/analyze/basic_block.rs |
Resolves lifted ghost receiver names. |
tests/ui/pass/ghost_self.rs |
Tests valid Self-typed ghost terms. |
tests/ui/fail/ghost_self.rs |
Tests invalid Self-typed ghost behavior. |
tests/ui/pass/ghost_generic.rs |
Tests valid generic ghost terms. |
tests/ui/fail/ghost_generic.rs |
Tests invalid generic ghost behavior. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d34fe77ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Follows #220, on top of #231. Three commits, each standing on its own:
4154ba3invariant!carried intoformula_fn_lifting— no behaviour changeeeb87fb2d34fe7ghost!, plus testsThe gap
A ghost term is lifted into a free
#[thrust::formula_fn], which inherits neither the enclosing function's generics norSelf, so a term could only name variables of concrete type — the last of #220's known gaps.#[thrust_macros::context]already threads that context intoinvariant!; it now threads it intoghost!too.How it works
formula_fn_liftingholds what both macros need to lift a formula out of a body:Selfis rewritten to theimpl's self type, or to a synthetic type parameter instantiated with the realSelfin a trait;selfis renamed to a__thrust_selfparameter, which the analyzer resolves back throughannot_fn::lifted_param_source_name.For
ghost!the introduced value is parameter0, so it passes through that lifting as an ordinary parameter: a value type namingSelfor a generic is rewritten along with the rest, while the__ghost_marker::<_, T>turbofish keeps the type as written, since the marker call stays in the host's scope.Tests
ghost_selfselfghost_genericEach as a
pass/failpair; in both, thefailfile differs only in which live variable the term names.Known gaps
Selfin a generic or traitimplis unsupported, unchanged frominvariant!— both now go through the sameTODO, so a fix covers them together.