Skip to content

Commit 5587463

Browse files
autoantwortclaude
andauthored
Fix #15094 SymbolDatabase: Variable::isReference() is true for function 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>
1 parent 16373cb commit 5587463

5 files changed

Lines changed: 124 additions & 1 deletion

File tree

‎lib/checkother.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1783,6 +1783,8 @@ void CheckOtherImpl::checkConstVariable()
17831783
continue;
17841784
if (var->isConst())
17851785
continue;
1786+
if (Tokenizer::isFunctionPointer(var->nameToken())) // reference to function
1787+
continue;
17861788
const Scope* scope = var->scope();
17871789
if (!scope)
17881790
continue;

‎lib/symboldatabase.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -917,7 +917,8 @@ void SymbolDatabase::createSymbolDatabaseCopyAndMoveConstructors()
917917
continue;
918918

919919
const Variable* firstArg = func->getArgumentVar(0);
920-
if (firstArg->type() == scope.definedType) {
920+
// skip references to functions: Foo(Foo& (&f)())
921+
if (firstArg->type() == scope.definedType && !Token::Match(firstArg->typeEndToken(), "&|&& %name%| ) (")) {
921922
if (firstArg->isRValueReference())
922923
func->type = FunctionType::eMoveConstructor;
923924
else if (firstArg->isReference() && !firstArg->isPointer())
@@ -2596,6 +2597,10 @@ void Variable::evaluate(const Settings& settings)
25962597
} else if (tok->str() == "&&") { // Before simplification, && isn't split up
25972598
setFlag(fIsRValueRef, true);
25982599
setFlag(fIsReference, true); // Set also fIsReference
2600+
} else if (tok->str() == "(" && Token::simpleMatch(tok->link(), ") (")) {
2601+
// a reference before the parentheses belongs to the return type of a function pointer: int& (*f)()
2602+
setFlag(fIsRValueRef, false);
2603+
setFlag(fIsReference, false);
25992604
}
26002605

26012606
if (tok->str() == "<" && tok->link())

‎test/testautovariables.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ class TestAutoVariables : public TestFixture {
133133
TEST_CASE(returnReference27);
134134
TEST_CASE(returnReference28);
135135
TEST_CASE(returnReference29);
136+
TEST_CASE(returnReference30); // #15094
136137
TEST_CASE(returnReferenceFunction);
137138
TEST_CASE(returnReferenceContainer);
138139
TEST_CASE(returnReferenceLiteral);
@@ -1772,6 +1773,23 @@ class TestAutoVariables : public TestFixture {
17721773
errout_str());
17731774
}
17741775

1776+
void returnReference30() // #15094
1777+
{
1778+
check("int& f1(int& (*fp)()) { return fp(); }\n"
1779+
"void f2(int& (*fp)()) { int& r = fp(); r = 2; }\n"
1780+
"struct D {\n"
1781+
" int& call() { return f(); }\n"
1782+
" int& (*f)();\n"
1783+
"};\n"
1784+
"const int& f3(const int& (*fp)(int)) { return fp(1); }\n"
1785+
"int& (*g)();\n"
1786+
"int& f4() { return g(); }\n"
1787+
"typedef int& (*FP)();\n"
1788+
"int& f5(FP fp) { return fp(); }\n"
1789+
"int* f6(int& (*fp)()) { return &fp(); }\n");
1790+
ASSERT_EQUALS("", errout_str());
1791+
}
1792+
17751793
void returnReferenceFunction() {
17761794
check("int& f(int& a) {\n"
17771795
" return a;\n"

‎test/testother.cpp‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4226,6 +4226,17 @@ class TestOther : public TestFixture {
42264226
" int select(DS &ds) { return select(ds, [](int) {}); }\n"
42274227
"};\n");
42284228
ASSERT_EQUALS("", errout_str());
4229+
4230+
check("int& g();\n" // reference to function
4231+
"int k();\n"
4232+
"int f1() { int& (&r)() = g; return r(); }\n"
4233+
"int f2() { int (&r)() = k; return r(); }\n"
4234+
"int& f3(int& (&f)()) { return f(); }\n"
4235+
"int f4(int&& (&f)()) { return f(); }\n"
4236+
"int f5(int (&f)()) { return f(); }\n"
4237+
"using FR = int&(&)();\n"
4238+
"void f6(FR f) { f(); }\n");
4239+
ASSERT_EQUALS("", errout_str());
42294240
}
42304241

42314242
void constParameterCallback() {

‎test/testsymboldatabase.cpp‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1603,6 +1603,66 @@ class TestSymbolDatabase : public TestFixture {
16031603
ASSERT(p->valueType()->originalTypeName == "ubFunctionPointer_fp");
16041604
ASSERT(p->valueType()->reference == Reference::None);
16051605
}
1606+
{ // function pointer returning a reference
1607+
GET_SYMBOL_DB("void foo(int& (*f)()) {}\n");
1608+
const Variable* const p = db->getVariableFromVarId(1);
1609+
ASSERT(!p->isReference());
1610+
ASSERT(p->isPointer());
1611+
ASSERT(p->valueType());
1612+
ASSERT(p->valueType()->pointer == 1);
1613+
}
1614+
{
1615+
GET_SYMBOL_DB("void foo(int&& (*f)()) {}\n");
1616+
const Variable* const p = db->getVariableFromVarId(1);
1617+
ASSERT(!p->isReference());
1618+
ASSERT(!p->isRValueReference());
1619+
}
1620+
{
1621+
GET_SYMBOL_DB("struct S { int& (*f[2])(); };\n");
1622+
const Variable* const p = db->getVariableFromVarId(1);
1623+
ASSERT(!p->isReference());
1624+
ASSERT(p->isArray());
1625+
}
1626+
{ // pointer to member function returning a reference
1627+
GET_SYMBOL_DB("struct S { int& g(); };\n"
1628+
"void foo(int& (S::*pm)()) {}\n");
1629+
const Variable* const p = db->getVariableFromVarId(1);
1630+
ASSERT(p);
1631+
ASSERT_EQUALS("pm", p->name());
1632+
ASSERT(!p->isReference());
1633+
}
1634+
{ // parentheses that are no function declarator
1635+
GET_SYMBOL_DB("void foo(int& (r)) {}\n");
1636+
const Variable* const p = db->functionScopes.front()->function->getArgumentVar(0);
1637+
ASSERT(p);
1638+
ASSERT(p->isReference());
1639+
}
1640+
{
1641+
GET_SYMBOL_DB("void foo(int& UNUSED(r)) {}\n");
1642+
const Variable* const p = db->functionScopes.front()->function->getArgumentVar(0);
1643+
ASSERT(p);
1644+
ASSERT(p->isReference());
1645+
}
1646+
{ // reference to function pointer
1647+
GET_SYMBOL_DB("void foo(int (*&f)()) {}\n");
1648+
const Variable* const p = db->getVariableFromVarId(1);
1649+
ASSERT(p->isReference());
1650+
ASSERT(p->isPointer());
1651+
ASSERT(p->valueType());
1652+
ASSERT(p->valueType()->reference == Reference::LValue);
1653+
}
1654+
{ // reference to function returning a reference
1655+
GET_SYMBOL_DB("void foo(int& (&f)()) {}\n");
1656+
const Variable* const p = db->getVariableFromVarId(1);
1657+
ASSERT(p->isReference());
1658+
ASSERT(!p->isRValueReference());
1659+
}
1660+
{
1661+
GET_SYMBOL_DB("void foo(int&& (&f)()) {}\n");
1662+
const Variable* const p = db->getVariableFromVarId(1);
1663+
ASSERT(p->isReference());
1664+
ASSERT(!p->isRValueReference());
1665+
}
16061666
}
16071667

16081668
void VariableValueTypeTemplate() {
@@ -2411,6 +2471,31 @@ class TestSymbolDatabase : public TestFixture {
24112471
ASSERT(db && ctor && ctor->type == FunctionType::eMoveConstructor);
24122472
ASSERT(ctor && ctor->retDef == nullptr);
24132473
}
2474+
{
2475+
GET_SYMBOL_DB("class Foo { Foo(Foo&& (*f)()); };\n");
2476+
const Function* ctor = tokenizer.tokens()->tokAt(3)->function();
2477+
ASSERT(db && ctor && ctor->type == FunctionType::eConstructor);
2478+
}
2479+
{
2480+
GET_SYMBOL_DB("class Foo { Foo(Foo& (&f)()); };\n");
2481+
const Function* ctor = tokenizer.tokens()->tokAt(3)->function();
2482+
ASSERT(db && ctor && ctor->type == FunctionType::eConstructor);
2483+
}
2484+
{
2485+
GET_SYMBOL_DB("class Foo { Foo(Foo&& (&f)()); };\n");
2486+
const Function* ctor = tokenizer.tokens()->tokAt(3)->function();
2487+
ASSERT(db && ctor && ctor->type == FunctionType::eConstructor);
2488+
}
2489+
{
2490+
GET_SYMBOL_DB("class Foo { Foo(Foo (&f)()); };\n");
2491+
const Function* ctor = tokenizer.tokens()->tokAt(3)->function();
2492+
ASSERT(db && ctor && ctor->type == FunctionType::eConstructor);
2493+
}
2494+
{
2495+
GET_SYMBOL_DB("class Foo { Foo(Foo& (&)()); };\n");
2496+
const Function* ctor = tokenizer.tokens()->tokAt(3)->function();
2497+
ASSERT(db && ctor && ctor->type == FunctionType::eConstructor);
2498+
}
24142499
{
24152500
GET_SYMBOL_DB("void f() { extern void f(); }\n");
24162501
ASSERT(db && db->scopeList.size() == 2);
@@ -9987,6 +10072,8 @@ class TestSymbolDatabase : public TestFixture {
998710072
ASSERT_EQUALS("signed long", typeOf("a = (long)32;\n", "("));
998810073
ASSERT_EQUALS("signed long", typeOf("a = (long int)32;\n", "("));
998910074
ASSERT_EQUALS("signed long long", typeOf("a = (long long)32;\n", "("));
10075+
ASSERT_EQUALS("signed int &", typeOf("void f(int& (*fp)()) { fp(); }\n", "( ) ;")); // #15094
10076+
ASSERT_EQUALS("signed int &&", typeOf("void f(int&& (*fp)()) { fp(); }\n", "( ) ;"));
999010077
ASSERT_EQUALS("long double", typeOf("a = (long double)32;\n", "("));
999110078
ASSERT_EQUALS("char", typeOf("a = static_cast<char>(32);\n", "("));
999210079
ASSERT_EQUALS("", typeOf("a = (unsigned x)0;\n", "("));

0 commit comments

Comments
 (0)