Fix #748 (Move the argument count check into Macro::getMacroParameters())

simplecpp.cpp

@@ -2204,10 +2204,35 @@ namespace simplecpp {
             return ~0U;
         }
 
+        /**
+         * Find the arguments of a call of this macro.
+         * @param nameTokInst    the macro name in the call
+         * @param calledInDefine the call is in a #define, so the arguments must be on the same line
+         * @return the tokens that delimit the arguments: the '(', each ',' between two arguments and the ')'.
+         *         Empty when the macro is not function-like.
+         * @throws wrongNumberOfParameters if a function-like macro has the wrong number of arguments.
+         *         This includes the case where the name is not followed by '(' (or is the last token),
+         *         so the caller must check for that first if it is not an error.
+         */
         std::vector<const Token *> getMacroParameters(const Token *nameTokInst, bool calledInDefine) const {
-            if (!nameTokInst->next || nameTokInst->next->op != '(' || !functionLike())
+            if (!functionLike())
                 return {};
+            std::vector<const Token *> parametertokens;
+            if (nameTokInst->next && nameTokInst->next->op == '(')
+                parametertokens = findArgumentDelimiters(nameTokInst, calledInDefine);
+
+            if (variadic) {
+                if (parametertokens.size() < args.size())
+                    throw wrongNumberOfParameters(nameTokInst->location, name());
+            } else {
+                if (parametertokens.size() != args.size() + (args.empty() ? 2U : 1U))
+                    throw wrongNumberOfParameters(nameTokInst->location, name());
+            }
+            return parametertokens;
+        }
 
+        /** The '(', each ',' between two arguments and the ')' of the call. nameTokInst is followed by '('. */
+        std::vector<const Token *> findArgumentDelimiters(const Token *nameTokInst, bool calledInDefine) const {
             std::vector<const Token *> parametertokens;
             parametertokens.emplace_back(nameTokInst->next);
             unsigned int par = 0U;
@@ -2296,26 +2321,14 @@ namespace simplecpp {
             const bool calledInDefine = (loc.fileIndex != nameTokInst->location.fileIndex ||
                                          loc.line < nameTokInst->location.line);
 
-            std::vector<const Token*> parametertokens1(getMacroParameters(nameTokInst, calledInDefine));
-
-            if (functionLike()) {
-                // No arguments => not macro expansion
-                if (nameTokInst->next && nameTokInst->next->op != '(') {
-                    output.push_back(new Token(nameTokInst->str(), loc));
-                    return nameTokInst->next;
-                }
-
-                // Parse macro-call
-                if (variadic) {
-                    if (parametertokens1.size() < args.size()) {
-                        throw wrongNumberOfParameters(nameTokInst->location, name());
-                    }
-                } else {
-                    if (parametertokens1.size() != args.size() + (args.empty() ? 2U : 1U))
-                        throw wrongNumberOfParameters(nameTokInst->location, name());
-                }
+            // No arguments => not macro expansion
+            if (functionLike() && nameTokInst->next && nameTokInst->next->op != '(') {
+                output.push_back(new Token(nameTokInst->str(), loc));
+                return nameTokInst->next;
             }
 
+            std::vector<const Token*> parametertokens1(getMacroParameters(nameTokInst, calledInDefine));
+
             // If macro call uses __COUNTER__ then expand that first
             TokenList tokensparams(files);
             std::vector<const Token *> parametertokens2;

test.cpp

@@ -1161,6 +1161,42 @@ static void define_invalid_3()
     TODO_ASSERT_EQUALS("", "file0,2,syntax_error,failed to expand 'R', Wrong number of parameters for macro 'R'.\n", toString(outputList));
 }
 
+static void define_wrong_number_of_arguments()
+{
+    {
+        simplecpp::OutputList outputList;
+        ASSERT_EQUALS("", preprocess("#define f(a,b) a+b\nf(1)\n", &outputList));
+        ASSERT_EQUALS("file0,2,syntax_error,failed to expand 'f', Wrong number of parameters for macro 'f'.\n", toString(outputList));
+    }
+    {
+        simplecpp::OutputList outputList;
+        ASSERT_EQUALS("", preprocess("#define f(a,b) a+b\nf(1,2,3)\n", &outputList));
+        ASSERT_EQUALS("file0,2,syntax_error,failed to expand 'f', Wrong number of parameters for macro 'f'.\n", toString(outputList));
+    }
+    {
+        simplecpp::OutputList outputList;
+        ASSERT_EQUALS("", preprocess("#define f(a,b,...) a+b\nf(1)\n", &outputList));
+        ASSERT_EQUALS("file0,2,syntax_error,failed to expand 'f', Wrong number of parameters for macro 'f'.\n", toString(outputList));
+    }
+    {
+        simplecpp::OutputList outputList;
+        ASSERT_EQUALS("\n1", preprocess("#define f(a,...) a __VA_ARGS__\nf(1)\n", &outputList));
+        ASSERT_EQUALS("", toString(outputList));
+    }
+    {
+        // a function-like macro name that is not followed by ( is not expanded
+        simplecpp::OutputList outputList;
+        ASSERT_EQUALS("\nf ;", preprocess("#define f(a) a\nf;\n", &outputList));
+        ASSERT_EQUALS("", toString(outputList));
+    }
+    {
+        // TODO: f takes no arguments, gcc reports an error
+        simplecpp::OutputList outputList;
+        TODO_ASSERT_EQUALS("", "\n1", preprocess("#define f() 1\nf(1)\n", &outputList));
+        TODO_ASSERT_EQUALS("file0,2,syntax_error,failed to expand 'f', Wrong number of parameters for macro 'f'.\n", "", toString(outputList));
+    }
+}
+
 static void define_invalid_4()
 {
     const char code[] = "#define X(...)\n"
@@ -5546,6 +5582,7 @@ static void runTests(int argc, char **argv, Input input)
     TEST_CASE(define_invalid_2);
     TEST_CASE(define_invalid_3);
     TEST_CASE(define_invalid_4);
+    TEST_CASE(define_wrong_number_of_arguments);
     TEST_CASE(define_define_1);
     TEST_CASE(define_define_2);
     TEST_CASE(define_define_3);