fix: make five upstream-parity resolution rules stricter or more exact - #295
Conversation
…lved A Python or Go method value whose receiver type the source cannot prove, such as `self.store.fetch` with an unannotated `store` or `obj.Fetch` with an external type, fell back to the only project method of that name. Upstream keeps that unique-or-drop discipline. The receiver may be a library object, so the port now returns no edge. Every receiver-scoped path is unchanged: imports, self and cls, typed fields, locals and annotations, class names, typed-base descendants, and Go local types and method expressions.
… built
`get().m()`, `X.getState().m()` and a destructured `X.getState()` bound to
an action of ANY exported call-initializer that holds an inline action
object. So `export const fake = otherFactory(() => ({ reset() {} }))` made
`fake.getState().reset()` a call into `fake::reset`. Upstream checks
Zustand provenance for selectors only.
Every form now needs a Zustand factory on the initializer's call path to
the action function. The factories are `create`, `createStore` and
`createWithEqualityFn` from zustand, zustand/vanilla or
zustand/traditional, the default zustand import, and a namespace import's
member. A selector additionally needs a hook factory. A new extractor
helper reads that path with the parser, following the same search that
finds the actions, so a comment, a string, a type argument or an argument
off the path never proves provenance. A wrapped hook such as
`createSelectors(create(...))` now binds its selectors too.
An #if or #elif condition was decided only when it was a literal, a single defined test or a bare name; upstream reads the same forms. So under `#if 1 || FLAG` a macro was never definitely visible and its calls bound to a same-named function. A small C preprocessor expression evaluator now decides the whole expression, with C precedence: - `&&` and `||` decide whenever one side does; - `?:` with an unknown condition is known only when both branches agree; - every other operator needs known operands; - a definitely undefined name reads 0. Unseen names, call-like `__has_include(...)`, character literals, overflow, division by zero and malformed text stay unknown. #if, #elif and object-like #define values are first spliced across backslash continuations, with each continuation line's comments removed.
Any `#ifndef X` whose next directive was `#define X`, or a fallback `#define X(`, forced its undecided branch active, as upstream's guardsItself does. So a feature-flag default (`#ifndef FEATURE` / `#define FEATURE` / `#define HOOK(x) ...` among other code) suppressed a real HOOK call that a build with -DFEATURE makes. A fallback `#ifndef MIN` / `#define MIN(a, b)` did the same, although a prior MIN from an unseen header or -D may be a wrapper that calls the function. A guard now has to be the whole file: - the test is the first code line; - the next directive is an empty `#define X`; - the matching `#endif`, with no `#else` or `#elif` at its depth, is the last code line. Lines that hold only comments do not count as code. Every other `#ifndef` stays undecided, and the call keeps its function.
Any constructor mentioning initializer_list made the whole overload set
decline, whatever the construction's form, as upstream does. So
`struct L { L(std::initializer_list<int>); L(int); }; L value(1);` had no
edge, although C++ calls L(int).
The resolver now reads the construction at the reference's anchor, the
initializer's own `(`, and accepts one form: `T x(<literal>, ...)`, where
the balanced argument text holds no `{` and the first argument is a
numeric, character or string literal, or true, false or nullptr. Such an
argument is never and never converts to an initializer_list, so
constructors whose first parameter is the list are dropped, and the rest
compete by arity as before. Everything else still declines: an
identifier such as an initializer_list variable, a braced argument or
lambda, brace or default initialization, or a list past the first
parameter.
These resolution changes alter persisted edges, so an index built at version 18 is reported outdated and rebuilt, and sync stays equal to index --force across the upgrade.
The 2026-10-02 entry records how #295 makes each finding the retroactive review kept at upstream behavior stricter or more exact, with upstream's line for comparison. The Current alignment KEEP-RUST bullet points to it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 545dd7b3f5
ℹ️ 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".
| Some(if state.defined == Some(false) { | ||
| Value::Int(0) | ||
| } else { | ||
| Value::Truth(state.value) |
There was a problem hiding this comment.
Preserve integer macro values in compound conditions
For #define VERSION 2 followed by #if VERSION == 2, record stores the replacement only as Some(true), this arm reconstructs it as Value::Truth, and Value::int() converts that truth to 1; the comparison therefore becomes definitively false instead of true or unknown. This can skip active macro definitions/includes and retain fabricated function-call edges, so preserve the numeric value or remain unresolved when its identity has been lost.
AGENTS.md reference: AGENTS.md:L59-L61
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #295 +/- ##
==========================================
+ Coverage 95.30% 95.31% +0.01%
==========================================
Files 161 161
Lines 99130 99752 +622
==========================================
+ Hits 94478 95083 +605
- Misses 4652 4669 +17
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Round 1 of the review found two #if defects. A macro's value is recorded only as a truth, yet it was read as the number 1, so `#define N 2` made `#if N == 1` true. A name's value is now a truth with no number: arithmetic and comparison on it are unknown, while a logical result (`!`, `&&`, `||`, `defined`) is still a real 0 or 1. Continuations were spliced after comment removal, so in `#if 1 || /* comment \` the backslash vanished with the comment. Translation phase 2 now runs on the raw-string-masked source before comments are removed, for directives and the guard's code-line scan alike. Each joined line moves to the line that started it and leaves an empty line behind, so line numbers stay put. That replaces the post-hoc continuation splicing.
The release record for #295: the release PR merge and its tag SHA, the workflow run, the published digest, and the black-box acceptance against v0.52.1.
Summary
The retroactive kirocodex review of the v1.6.1 port (see #291) left five findings at upstream behavior. The owner asked on 2026-10-02 for all five to be fixed beyond upstream
v1.6.1(f4ddf50). The design passed the kirocodex plan review in round 3 (blocking items 6 → 1 → 0). Each fix is a KEEP-RUST divergence, stricter than upstream or more exact, and each was red first.7f9cc86self.store.fetchwith an unannotatedstore, orobj.Fetchwith an external type) leaves the value unresolved. Upstream falls back to the only project method of that name.3cafab6get(),X.getState(), a destructuredgetState()and selectors bind only inside a store a Zustand factory built. The factory must sit on the initializer's call path to the action function. A new extractor helper reads that path with the parser, following the same search that finds the actions, so a comment, a string, a type argument or an argument off the path never proves provenance. A wrapped hook (createSelectors(create(...))) now binds its selectors.e69f39d#if#if/#elifthree-valued, with C precedence.&&/`405ab71#define X, and an#endifthat is the last code line. A feature-flag default and a fallback#ifndef MIN/#define MIN(a, b)stay undecided, because a priorMINmay call the function, so the call keeps its edge.8e44db7T x(<literal>, ...), with no{in the arguments, drops the constructors whose first parameter is theinitializer_list, then resolves by arity. A variable argument, braces and every other form still decline.545dd7bVerification
/tmp/evidence-p10/red-{A,B,C,D,E}-*.logrecord each new test failing on its base for the stated reason.identical.make pre-ciat545dd7b, clean tree, rustc 1.98.0: 3981 passed, 0 failed, all checks passed./tmp/verify-0.52.2.sh, the plan's acceptance table, ran against this head's release build and the official v0.52.1. Every row matches:fake::resetis gone anduseStore::resetis kept;HOOKedge is gone;HOOK2edge appears;L::Ledge appears;outdated, thencurrentaftersync.BEGIN_COMMIT_OVERRIDE
fix(resolve): leave a method value with an unknowable receiver unresolved
fix(resolve): bind store actions only inside stores a Zustand factory built
fix(resolve): evaluate whole #if expressions three-valued
fix(resolve): treat only a whole-file #ifndef as an include guard
fix(resolve): construct from a literal past initializer_list overloads
END_COMMIT_OVERRIDE
🤖 Generated with Claude Code