could_be_immutable: fix false positives for ref/out parameters and member access - #978
Open
cschlote wants to merge 1 commit into
Open
Conversation
…mber access The check treated simple value types passed to a function as unmodified, but the argument may be bound to a `ref` or `out` parameter, which modifies it regardless of its type. Treat function call and new-expression arguments as potential modifications. Member accesses (`a.b`) were only considered when they appeared in an otherwise interesting context, but a member can be a non-const method or property (e.g. range accessors like `empty`/`front`) or return a mutable copy that requires a mutable base (e.g. `Nullable.get` on a struct holding an array). Without semantic analysis, const-ness of the base cannot be proven, so treat the base of a member access as potentially modified. Index expressions, return statements and bare reads of value types are still reported as before.
WebFreak001
requested changes
Aug 6, 2026
| immutable id = produce(createdNow); | ||
| return id + (createdNow ? 1 : 0); | ||
| } | ||
| }, sac); |
Member
There was a problem hiding this comment.
please add a test showing that a regular function parameter won't suppress the lint (out and ref parameters are not that common, they shouldn't be assumed to always be there and break the const lint)
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.
Fixes #366, fixes #325 (also covers the case reported in #298).
The check suggests
const/immutabledeclarations that do not compile in two situations.1. Arguments bound to
ref/outparameters (#298 / #366)D-Scanner cannot know
mulu's signature (see the discussion in #366: "this limitation is unlikely to fall"), so a by-value and aref/outbinding cannot be distinguished. Arguments of function calls andnewexpressions are therefore now treated as potential modifications regardless of their type (newcallArgumentcounter disabling the value-type early return invariableMightBeModified). The #640 behavior (new Foo(i1)does not warn) is preserved.2. Member access without a call (#325)
As noted in #325, the AST cannot tell whether
d.popFrontis a call, and the member might be a non-constmethod/property (range accessors) or return something that requires a mutable base — e.g.Nullable.geton a struct holding an array, whereimmutable docfails with "contains pointers or references". The base of any member access (a.b, chaineda.b.c) is now treated as potentially modified (newmarkMemberAccessBase).Preserved behavior / trade-off
int i = 1; return i;int i = 0; return arr[i];Point p; return p.x;) — member const-ness cannot be proven syntactically.Testing
unmodified.d, plus regression tests for the preserved warnings.make test(54 modules) andtests/it.shpass.could_be_immutablewarnings — all confirmed false positives, e.g. theequalRangeresult range inunmodified.ditself — and adds none.