fix #11798: FP missingReturn on function with switch (#6806)

danmar · Oct 5, 2024 · 88a236c · 88a236c
1 parent addfb01
commit 88a236c
Show file tree

Hide file tree

Showing 4 changed files with 144 additions and 7 deletions.
diff --git a/lib/astutils.cpp b/lib/astutils.cpp
@@ -3664,3 +3664,61 @@ bool isUnevaluated(const Token *tok)
 {
     return Token::Match(tok, "alignof|_Alignof|_alignof|__alignof|__alignof__|decltype|offsetof|sizeof|typeid|typeof|__typeof__ (");
 }
+
+static std::set<MathLib::bigint> getSwitchValues(const Token *startbrace, bool &hasDefault)
+{
+    std::set<MathLib::bigint> values;
+    const Token *endbrace = startbrace->link();
+    if (!endbrace)
+        return values;
+
+    hasDefault = false;
+    for (const Token *tok = startbrace->next(); tok && tok != endbrace; tok = tok->next()) {
+        if (Token::simpleMatch(tok, "{") && tok->scope()->type == Scope::ScopeType::eSwitch) {
+            tok = tok->link();
+            continue;
+        }
+        if (Token::simpleMatch(tok, "default")) {
+            hasDefault = true;
+            break;
+        }
+        if (Token::simpleMatch(tok, "case")) {
+            const Token *valueTok = tok->astOperand1();
+            if (valueTok->hasKnownIntValue())
+                values.insert(valueTok->getKnownIntValue());
+            continue;
+        }
+    }
+
+    return values;
+}
+
+bool isExhaustiveSwitch(const Token *startbrace)
+{
+    if (!startbrace || !Token::simpleMatch(startbrace->previous(), ") {") || startbrace->scope()->type != Scope::ScopeType::eSwitch)
+        return false;
+    const Token *rpar = startbrace->previous();
+    const Token *lpar = rpar->link();
+
+    const Token *condition = lpar->astOperand2();
+    if (!condition->valueType())
+        return true;
+
+    bool hasDefault = false;
+    const std::set<MathLib::bigint> switchValues = getSwitchValues(startbrace, hasDefault);
+
+    if (hasDefault)
+        return true;
+
+    if (condition->valueType()->type == ValueType::Type::BOOL)
+        return switchValues.count(0) && switchValues.count(1);
+
+    if (condition->valueType()->isEnum()) {
+        const std::vector<Enumerator> &enumList = condition->valueType()->typeScope->enumeratorList;
+        return std::all_of(enumList.cbegin(), enumList.cend(), [&](const Enumerator &e) {
+            return !e.value_known || switchValues.count(e.value);
+        });
+    }
+
+    return false;
+}
diff --git a/lib/astutils.h b/lib/astutils.h
@@ -446,4 +446,6 @@ bool isGlobalData(const Token *expr);
 
 bool isUnevaluated(const Token *tok);
 
+bool isExhaustiveSwitch(const Token *startbrace);
+
 #endif // astutilsH
diff --git a/lib/checkfunctions.cpp b/lib/checkfunctions.cpp
@@ -359,8 +359,6 @@ static const Token *checkMissingReturnScope(const Token *tok, const Library &lib
                     return nullptr;
             }
             if (tok->scope()->type == Scope::ScopeType::eSwitch) {
-                // find reachable break / !default
-                bool hasDefault = false;
                 bool reachable = false;
                 for (const Token *switchToken = tok->link()->next(); switchToken != tok; switchToken = switchToken->next()) {
                     if (reachable && Token::simpleMatch(switchToken, "break ;")) {
@@ -375,12 +373,10 @@ static const Token *checkMissingReturnScope(const Token *tok, const Library &lib
                         reachable = false;
                     if (Token::Match(switchToken, "case|default"))
                         reachable = true;
-                    if (Token::simpleMatch(switchToken, "default :"))
-                        hasDefault = true;
                     else if (switchToken->str() == "{" && (switchToken->scope()->isLoopScope() || switchToken->scope()->type == Scope::ScopeType::eSwitch))
                         switchToken = switchToken->link();
                 }
-                if (!hasDefault)
+                if (!isExhaustiveSwitch(tok->link()))
                     return tok->link();
             } else if (tok->scope()->type == Scope::ScopeType::eIf) {
                 const Token *condition = tok->scope()->classDef->next()->astOperand2();

diff --git a/test/testfunctions.cpp b/test/testfunctions.cpp
@@ -74,7 +74,11 @@ class TestFunctions : public TestFixture {
         TEST_CASE(memsetInvalid2ndParam);
 
         // missing "return"
-        TEST_CASE(checkMissingReturn);
+        TEST_CASE(checkMissingReturn1);
+        TEST_CASE(checkMissingReturn2); // #11798
+        TEST_CASE(checkMissingReturn3);
+        TEST_CASE(checkMissingReturn4);
+        TEST_CASE(checkMissingReturn5);
 
         // std::move for locar variable
         TEST_CASE(returnLocalStdMove1);
@@ -1556,7 +1560,7 @@ class TestFunctions : public TestFixture {
         ASSERT_EQUALS("[test.cpp:4]: (portability) The 2nd memset() argument '1.0f+i' is a float, its representation is implementation defined.\n", errout_str());
     }
 
-    void checkMissingReturn() {
+    void checkMissingReturn1() {
         check("int f() {}");
         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());
 
@@ -1764,6 +1768,83 @@ class TestFunctions : public TestFixture {
         ASSERT_EQUALS("", errout_str());
     }
 
+    void checkMissingReturn2() { // #11798
+        check("int f(bool const a) {\n"
+              "      switch (a) {\n"
+              "      case true:\n"
+              "            return 1;\n"
+              "      case false:\n"
+              "            return 2;\n"
+              "      }\n"
+              "}\n"
+              "int _tmain(int argc, _TCHAR* argv[])\n"
+              "{\n"
+              "      auto const b= f(true);\n"
+              "      auto const c= f(false);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+    }
+
+    void checkMissingReturn3() {
+        check("enum Enum {\n"
+              "    A,\n"
+              "    B,\n"
+              "    C,\n"
+              "};\n"
+              "int f(Enum e) {\n"
+              "    switch (e) {\n"
+              "    case A:\n"
+              "          return 1;\n"
+              "    case B:\n"
+              "          return 2;\n"
+              "    case C:\n"
+              "          return 2;\n"
+              "    }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+    }
+
+    void checkMissingReturn4() {
+        check("enum Enum {\n"
+              "    A,\n"
+              "    B,\n"
+              "    C,\n"
+              "};\n"
+              "int f(Enum e) {\n"
+              "    switch (e) {\n"
+              "    case A:\n"
+              "          return 1;\n"
+              "    case B:\n"
+              "          return 2;\n"
+              "    }\n"
+              "}\n");
+        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());
+    }
+
+    void checkMissingReturn5() {
+        check("enum Enum {\n"
+              "    A,\n"
+              "    B,\n"
+              "    C,\n"
+              "};\n"
+              "int f(Enum e, bool b) {\n"
+              "    switch (e) {\n"
+              "    case A:\n"
+              "          return 1;\n"
+              "    case B:\n"
+              "          return 2;\n"
+              "    case C:\n"
+              "          switch (b) {\n"
+              "          case true:\n"
+              "              return 3;\n"
+              "          case false:\n"
+              "              return 4;\n"
+              "          }\n"
+              "    }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+    }
+
     // NRVO check
     void returnLocalStdMove1() {
         check("struct A{}; A f() { A var; return std::move(var); }");