Fix #15021 FN redundantInitialization with structured binding

[chrchr-github]

Best viewed with whitespace changes hidden.

[danmar]

This is an AI review. Take it with a grain of salt and feel free to reject it by resolving the comment.

Looks good to me. I built it and checked that reference bindings don't produce false positives: auto& [x, y] = s; x = 1;, auto&& [x, y] = std::move(s);, for (auto& [k, v] : m) v = 0; and auto& [x, y] = arr; all stay silent. auto [x, y] = p; with a std::pair is reported.

Question: the ticket and the PR title say "FN redundantInitialization", but since isInitialization is only set for split ; %var% = declarations, the structured binding is reported as redundantAssignment ("is reassigned a value before the old one has been used", with notes "x is assigned" / "x is overwritten"). Semantically auto [x, y] = s; is an initialization. Should isInitialization be set for the structured binding case so that redundantInitialization is reported, including the existing "trivial rhs" handling? If redundantAssignment is intended, maybe adjust the title so the history matches.

[dmcppcheck]

Test results for commit 7ae3dd4 (tools/test-my-pr.py, main compared to this PR):

Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8847/
packages: 100
Differing warnings: 0 (+0 -0)
Timing: -0.0% (http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8847/2026-09-30_08-54-05-7ae3dd410277_timing.html)

_{Posted automatically by the cppcheck PR test runner. +N: warnings only with this PR, -N: warnings only with main. The AI review is written by Claude and can be wrong.}

[danmar]

I didn't read the AI review carefully.. if you feel we can ignore that then feel free to merge this.

[dmcppcheck]

This pull request has merge conflicts that must be resolved.

[dmcppcheck]

@claude review

[claude]

Possible FP with reference bindings: the isReference() bailout above (// todo: check references) runs on tok->astOperand1(), which is the [ token here and has no variable. So it never applies to the bound names. With auto& [x, y] = s; x = 1;, x is an alias for s.a and the write is observable, so it isn't redundant. The same applies to tuple-like types whose elements are references (e.g. auto [a, b] = std::tie(i, j);).

I'd suggest skipping elements whose variable()->isReference() is true (or bailing out when the declaration is auto&/auto&&), and adding a negative test for auto& [x, y] = s; x = 1; y = 2;.

[claude]

Now that this runs inside the loop, there are two problems for structured bindings:

• diag(tok) inserts tok the first time, so only the first bound element can ever get this diagnostic.
• The message still uses tok->astOperand1()->expressionString(), which would print [x,y] instead of the element name. It should use exprTok/tokenToCheck, like the other diagnostics below.

[claude]

Review summary:

• Possible FP: reference bindings (auto& [x, y] = s; x = 1;) aren't excluded. The existing isReference() bailout checks the [ token, not the bound variables. A negative test for this would help.
• Same-value diagnostic in the loop: diag(tok) limits it to the first bound element, and the message uses [x,y] instead of the element name.
• Title vs. test: the title says redundantInitialization, but the new test only expects redundantAssignment. Is that intended?

I couldn't build locally, so these come from reading the code.

🤖 Generated with Claude Code

[dmcppcheck]

Test results for commit 67b4065 (tools/test-my-pr.py, main compared to this PR):

Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8847/
packages: 100
Differing warnings: 0 (+0 -0)
Timing: +0.0% (http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8847/2026-10-10_18-08-53-67b40654588d_timing.html)

_{Posted automatically by the cppcheck PR test runner. +N: warnings only with this PR, -N: warnings only with main. The AI review is written by Claude and can be wrong.}

[dmcppcheck]

AI review

The two bailouts that precede the new loop look only at tok->astOperand1(), which is the [ token for a structured binding, so they never apply to the bound names. Both effects are reproduced:

• A bound element whose class has a user-defined operator= is reported as a normal (non-inconclusive) redundantInitialization without --inconclusive. The equivalent plain variable is only reported with --inconclusive.
• A bound std::shared_ptr element is reported, while the equivalent plain variable is deliberately never reported ("assigning a smart pointer has side effects").

The reference-element false positives already raised in the earlier review (std::tie, struct with reference members) still reproduce. I found nothing else new; static auto [..], auto&&, lambda capture, address-taken, if/for init-statements, goto, templates and early returns behaved sensibly.

Testing

Built cppcheck and testrunner in pr/ with make -j8. ./testrunner TestOther passes (0 failed). base/ was not built, so there is no before/after comparison; plain-variable equivalents in the same file serve as the reference instead.

Problem: the inconclusive and smart-pointer bailouts are skipped for bound names

t3.cpp:

#include <memory>
struct A { A(); A& operator=(const A&); int v; };
struct SA { A a; A b; };
void use(const A&);
void f1(SA s) { auto [x, y] = s; x = A(); use(x); }
void f2(SA s) { A x = s.a; x = A(); use(x); }
struct SP { std::shared_ptr<int> p; int q; };
int f3(SP s) { auto [p, q] = s; p = std::make_shared<int>(1); return *p + q; }
int f4(SP s) { std::shared_ptr<int> p = s.p; p = std::make_shared<int>(1); return *p; }

$ cppcheck -q --enable=style --library=std --template='{line}: {id}: {message}' t3.cpp
5: redundantInitialization: Redundant initialization for 'x'. The initialized value is overwritten before it is read.
8: redundantInitialization: Redundant initialization for 'p'. The initialized value is overwritten before it is read.

With --inconclusive --template='{line}: {id}: {message} [{inconclusive:inconclusive}]':

5: redundantInitialization: ... 'x' ... []
6: redundantInitialization: ... 'x' ... [inconclusive]
8: redundantInitialization: ... 'p' ... []

Line 5 (binding) is reported as conclusive, line 6 (plain variable) only as inconclusive. Line 8 (binding of a shared_ptr) is reported, line 9 (plain variable) never is.

Other cases run (--enable=style --inconclusive --library=std):

• Not reported, as expected: static auto [x, y] = s; x = 1;, auto&& [x, y] = s; x = 1;, a bound name captured by a lambda or with its address taken before the write, if (auto [x, y] = s; y) { x = 1; }, for (auto [i, j] = get(); i < j; i = i + 1), a write after goto, a write after a conditional return x, a write inside try or switch.
• Reported, and correct: binding from an array, from *s, from a function call, from std::pair/std::make_pair, inside a template, and auto [it, ok] = m.emplace(1, 2); it = m.begin(); (same as the plain-variable form).
• Not reported (false negative, no regression): auto [x, y]{s}; and auto [x, y](s);.
• Still reported, already raised in the earlier review: auto [a, b] = std::tie(i, j); a = 1; and struct R { int& a; int b; }; auto [x, y] = r; x = 1;.

---

Automated review of 97cd12e by Claude. The pull request was built and tested locally.

[dmcppcheck]

AI review

The checks above this point (custom operator= => inconclusive, SMART_POINTER => bail out) use tok->astOperand1()->valueType(), which is the [ token for a structured binding, so they never apply to the bound names. Reproduced:

struct A { A(); A& operator=(const A&); int v; };
struct SA { A a; A b; };
void use(const A&);
void f1(SA s) { auto [x, y] = s; x = A(); use(x); }   // reported without --inconclusive
void f2(SA s) { A x = s.a; x = A(); use(x); }         // only reported with --inconclusive

struct SP { std::shared_ptr<int> p; int q; };
int f3(SP s) { auto [p, q] = s; p = std::make_shared<int>(1); return *p + q; }               // reported
int f4(SP s) { std::shared_ptr<int> p = s.p; p = std::make_shared<int>(1); return *p; }      // not reported

The valueType() based checks should be done per element inside the loop, using tokenToCheck->valueType(), so that bindings get the same treatment as plain variables. Please add tests for both cases.

[dmcppcheck]

Test results for commit 97cd12e (tools/test-my-pr.py, main compared to this PR):

Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8847/
packages: 100
Differing warnings: 0 (+0 -0)
Timing: -0.2% (http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8847/2026-10-12_18-24-33-97cd12e396e6_timing.html)

_{Posted automatically by the cppcheck PR test runner. +N: warnings only with this PR, -N: warnings only with main. The AI review is written by Claude and can be wrong.}