Skip to content

Commit 88a236c

Browse files
ludviggunneludviggunne
authored
fix #11798: FP missingReturn on function with switch (cppcheck-opensource#6806)
1 parent addfb01 commit 88a236c

4 files changed

Lines changed: 144 additions & 7 deletions

File tree

‎lib/astutils.cpp‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3664,3 +3664,61 @@ bool isUnevaluated(const Token *tok)
36643664
{
36653665
return Token::Match(tok, "alignof|_Alignof|_alignof|__alignof|__alignof__|decltype|offsetof|sizeof|typeid|typeof|__typeof__ (");
36663666
}
3667+
3668+
static std::set<MathLib::bigint> getSwitchValues(const Token *startbrace, bool &hasDefault)
3669+
{
3670+
std::set<MathLib::bigint> values;
3671+
const Token *endbrace = startbrace->link();
3672+
if (!endbrace)
3673+
return values;
3674+
3675+
hasDefault = false;
3676+
for (const Token *tok = startbrace->next(); tok && tok != endbrace; tok = tok->next()) {
3677+
if (Token::simpleMatch(tok, "{") && tok->scope()->type == Scope::ScopeType::eSwitch) {
3678+
tok = tok->link();
3679+
continue;
3680+
}
3681+
if (Token::simpleMatch(tok, "default")) {
3682+
hasDefault = true;
3683+
break;
3684+
}
3685+
if (Token::simpleMatch(tok, "case")) {
3686+
const Token *valueTok = tok->astOperand1();
3687+
if (valueTok->hasKnownIntValue())
3688+
values.insert(valueTok->getKnownIntValue());
3689+
continue;
3690+
}
3691+
}
3692+
3693+
return values;
3694+
}
3695+
3696+
bool isExhaustiveSwitch(const Token *startbrace)
3697+
{
3698+
if (!startbrace || !Token::simpleMatch(startbrace->previous(), ") {") || startbrace->scope()->type != Scope::ScopeType::eSwitch)
3699+
return false;
3700+
const Token *rpar = startbrace->previous();
3701+
const Token *lpar = rpar->link();
3702+
3703+
const Token *condition = lpar->astOperand2();
3704+
if (!condition->valueType())
3705+
return true;
3706+
3707+
bool hasDefault = false;
3708+
const std::set<MathLib::bigint> switchValues = getSwitchValues(startbrace, hasDefault);
3709+
3710+
if (hasDefault)
3711+
return true;
3712+
3713+
if (condition->valueType()->type == ValueType::Type::BOOL)
3714+
return switchValues.count(0) && switchValues.count(1);
3715+
3716+
if (condition->valueType()->isEnum()) {
3717+
const std::vector<Enumerator> &enumList = condition->valueType()->typeScope->enumeratorList;
3718+
return std::all_of(enumList.cbegin(), enumList.cend(), [&](const Enumerator &e) {
3719+
return !e.value_known || switchValues.count(e.value);
3720+
});
3721+
}
3722+
3723+
return false;
3724+
}

‎lib/astutils.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,4 +446,6 @@ bool isGlobalData(const Token *expr);
446446

447447
bool isUnevaluated(const Token *tok);
448448

449+
bool isExhaustiveSwitch(const Token *startbrace);
450+
449451
#endif // astutilsH

‎lib/checkfunctions.cpp‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -359,8 +359,6 @@ static const Token *checkMissingReturnScope(const Token *tok, const Library &lib
359359
return nullptr;
360360
}
361361
if (tok->scope()->type == Scope::ScopeType::eSwitch) {
362-
// find reachable break / !default
363-
bool hasDefault = false;
364362
bool reachable = false;
365363
for (const Token *switchToken = tok->link()->next(); switchToken != tok; switchToken = switchToken->next()) {
366364
if (reachable && Token::simpleMatch(switchToken, "break ;")) {
@@ -375,12 +373,10 @@ static const Token *checkMissingReturnScope(const Token *tok, const Library &lib
375373
reachable = false;
376374
if (Token::Match(switchToken, "case|default"))
377375
reachable = true;
378-
if (Token::simpleMatch(switchToken, "default :"))
379-
hasDefault = true;
380376
else if (switchToken->str() == "{" && (switchToken->scope()->isLoopScope() || switchToken->scope()->type == Scope::ScopeType::eSwitch))
381377
switchToken = switchToken->link();
382378
}
383-
if (!hasDefault)
379+
if (!isExhaustiveSwitch(tok->link()))
384380
return tok->link();
385381
} else if (tok->scope()->type == Scope::ScopeType::eIf) {
386382
const Token *condition = tok->scope()->classDef->next()->astOperand2();

‎test/testfunctions.cpp‎

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ class TestFunctions : public TestFixture {
7474
TEST_CASE(memsetInvalid2ndParam);
7575

7676
// missing "return"
77-
TEST_CASE(checkMissingReturn);
77+
TEST_CASE(checkMissingReturn1);
78+
TEST_CASE(checkMissingReturn2); // #11798
79+
TEST_CASE(checkMissingReturn3);
80+
TEST_CASE(checkMissingReturn4);
81+
TEST_CASE(checkMissingReturn5);
7882

7983
// std::move for locar variable
8084
TEST_CASE(returnLocalStdMove1);
@@ -1556,7 +1560,7 @@ class TestFunctions : public TestFixture {
15561560
ASSERT_EQUALS("[test.cpp:4]: (portability) The 2nd memset() argument '1.0f+i' is a float, its representation is implementation defined.\n", errout_str());
15571561
}
15581562

1559-
void checkMissingReturn() {
1563+
void checkMissingReturn1() {
15601564
check("int f() {}");
15611565
ASSERT_EQUALS("[test.cpp:1]: (error) Found an exit path from function with non-void return type that has missing return statement\n", errout_str());
15621566

@@ -1764,6 +1768,83 @@ class TestFunctions : public TestFixture {
17641768
ASSERT_EQUALS("", errout_str());
17651769
}
17661770

1771+
void checkMissingReturn2() { // #11798
1772+
check("int f(bool const a) {\n"
1773+
" switch (a) {\n"
1774+
" case true:\n"
1775+
" return 1;\n"
1776+
" case false:\n"
1777+
" return 2;\n"
1778+
" }\n"
1779+
"}\n"
1780+
"int _tmain(int argc, _TCHAR* argv[])\n"
1781+
"{\n"
1782+
" auto const b= f(true);\n"
1783+
" auto const c= f(false);\n"
1784+
"}\n");
1785+
ASSERT_EQUALS("", errout_str());
1786+
}
1787+
1788+
void checkMissingReturn3() {
1789+
check("enum Enum {\n"
1790+
" A,\n"
1791+
" B,\n"
1792+
" C,\n"
1793+
"};\n"
1794+
"int f(Enum e) {\n"
1795+
" switch (e) {\n"
1796+
" case A:\n"
1797+
" return 1;\n"
1798+
" case B:\n"
1799+
" return 2;\n"
1800+
" case C:\n"
1801+
" return 2;\n"
1802+
" }\n"
1803+
"}\n");
1804+
ASSERT_EQUALS("", errout_str());
1805+
}
1806+
1807+
void checkMissingReturn4() {
1808+
check("enum Enum {\n"
1809+
" A,\n"
1810+
" B,\n"
1811+
" C,\n"
1812+
"};\n"
1813+
"int f(Enum e) {\n"
1814+
" switch (e) {\n"
1815+
" case A:\n"
1816+
" return 1;\n"
1817+
" case B:\n"
1818+
" return 2;\n"
1819+
" }\n"
1820+
"}\n");
1821+
ASSERT_EQUALS("[test.cpp:7]: (error) Found an exit path from function with non-void return type that has missing return statement\n", errout_str());
1822+
}
1823+
1824+
void checkMissingReturn5() {
1825+
check("enum Enum {\n"
1826+
" A,\n"
1827+
" B,\n"
1828+
" C,\n"
1829+
"};\n"
1830+
"int f(Enum e, bool b) {\n"
1831+
" switch (e) {\n"
1832+
" case A:\n"
1833+
" return 1;\n"
1834+
" case B:\n"
1835+
" return 2;\n"
1836+
" case C:\n"
1837+
" switch (b) {\n"
1838+
" case true:\n"
1839+
" return 3;\n"
1840+
" case false:\n"
1841+
" return 4;\n"
1842+
" }\n"
1843+
" }\n"
1844+
"}\n");
1845+
ASSERT_EQUALS("", errout_str());
1846+
}
1847+
17671848
// NRVO check
17681849
void returnLocalStdMove1() {
17691850
check("struct A{}; A f() { A var; return std::move(var); }");

0 commit comments

Comments
 (0)