Repository navigation
Fix #15080 FP uninitMemberVarNoCtor for const and reference members - #8910
Conversation
A class without constructor whose non-static const or reference member has no default member initializer is not default constructible (or, for a const class type with a default constructor, default constructs it), so the member can never be left uninitialized. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…or for all members Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Any further changes requested? |
|
@claude review |
|
The approach makes sense: a const member of trivial type, or a reference member, with no initializer deletes the implicit default constructor, so every instance has to be initialized another way. One issue (inline): static const and static reference members are in the usage list too. The new check doesn't exclude them, so a declaration like Minor: it would be good to add a test for a const pointer member ( 🤖 Generated with Claude Code |
|
please check the results here: |
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Test results for commit d9ff8a4 (tools/test-my-pr.py, main compared to this PR): Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8910/ 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. |
It is so cool to have a real would test of the changes and an automatic review of the differences |
Variable::isReference() is also true for `std::string& (*f)(int)`, which suppressed uninitMemberVarNoCtor for the whole class (codelite in the PR test results). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the real-world run! I checked the 8 regressions + 1 unclear:
(Written by Claude Code on behalf of @autoantwort.) |
| // isReference() is also true for a function pointer that returns a reference | ||
| const bool isReferenceMember = var.isReference() && var.nameToken() && Token::Match(var.nameToken()->previous(), "&|&&"); |
There was a problem hiding this comment.
I think a better fix would be to fix isReference
There was a problem hiding this comment.
Claude creates a new PR for this,
|
@claude review |
|
Re-reviewed after fc4b65a. The earlier points are addressed: there is now a I found no correctness bugs in the current diff:
There is one remaining false positive. It is not a regression, and a follow-up is fine. A const member of an aggregate class type with no user-provided default constructor, such as 🤖 Generated with Claude Code |
I think a follow up MR is better then making this MR bigger |
|
Test results for commit fc4b65a (tools/test-my-pr.py, main compared to this PR): Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8910/ 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
left a comment
There was a problem hiding this comment.
to me it feels like this PR has evolved into something great
- well tested
- It looks like the changes in the checker are limited.
of course it's approved. :-)
…on pointer returning a reference (#8931) https://trac.cppcheck.net/ticket/15094 For a function pointer returning a reference, the reference of the return type was taken as reference of the variable: ```cpp struct S { std::string& (*f)(int, int); int& (*g[2])(); }; void foo(int& (*fp)(), int&& (*fp2)()); ``` `Variable::isReference()` (and `isRValueReference()` for `fp2`) was true for all of them. `Variable::evaluate()` now drops the reference collected before the `(` of a function declarator, as it belongs to the return type. A `&` after the `(` still makes the variable a reference, so `int (*&f)()`, `int (&f)()`, `int*& r`, `int (&a)[2]` and `int& (r)` are unchanged. The `ValueType` of the function pointer is not changed: the type of a call `fp()` is derived from it, so its reference has to stay the one of the return type (clearing it caused `returnTempReference` FPs). Two places relied on the wrong flags and are adjusted: - `checkConstVariable`: a reference to a function returning a reference (`int& (&f)()`) was skipped only because it was wrongly flagged as rvalue reference. References to functions are now skipped explicitly (this also fixes the existing FP for `int (&f)()`). - Copy/move constructor detection: `Foo(Foo&& (*f)())` was a move constructor. With the fix it is a normal constructor, and parameters that are references to functions (`Foo(Foo& (&f)())`) no longer make a copy/move constructor either. Found with #8910, where it suppressed `uninitMemberVarNoCtor` for a struct with such a member (codelite in the PR test results). #8910 contains a workaround in CheckClass for this; with this PR merged it can be replaced by `var.isReference()` again. This PR was created by Claude Code (an AI coding agent) on behalf of @autoantwort. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
https://trac.cppcheck.net/ticket/15080
gives
Member variable 'S::it' has no initializer. [uninitMemberVarNoCtor], andstruct S { int a = 0; const int b; int& c; };warns forbandc.A non-static const or reference member without default member initializer can't stay uninitialized: for trivial types the implicit default constructor is deleted, so every instance must initialize it, and a const class type is default-constructed. These members are now skipped by the no-constructor check.
Since f90a76f (#15072) this also shows up for class templates whose member type is a
usingalias.This PR was created by Claude Code (an AI coding agent) on behalf of @autoantwort.
🤖 Generated with Claude Code