preprocessor: reject a malformed macro call instead of silently accepting

Address review feedback: when the macro-argument scan reaches an
unexpected token (in particular the EOF from an unterminated call),
ConsumeAndParseMacroCall now returns an InvalidArgumentError and the
caller records it as a preprocessor error at the call site, rather than
silently back-filling the arguments. An early ')' remains the legal way
to pass fewer arguments than parameters.

Signed-off-by: Eylon Krause <eylon1909@gmail.com>

verible/verilog/preprocessor/verilog-preprocess.cc

diff --git a/verible/verilog/preprocessor/verilog-preprocess.cc b/verible/verilog/preprocessor/verilog-preprocess.cc
index d8f5f5b..9c27df6 100644
--- a/verible/verilog/preprocessor/verilog-preprocess.cc
+++ b/verible/verilog/preprocessor/verilog-preprocess.cc
@@ -270,10 +270,14 @@
     }
     // Any other token -- in particular the EOF token from an unterminated
     // macro call -- would otherwise leave token_iter and parameters_size
-    // unchanged and spin this loop forever. Stop scanning; the loop below
-    // back-fills the remaining parameters with default TokenInfo (the same
-    // terminal state produced by an early ')').
-    break;
+    // unchanged and spin this loop forever. An early ')' (handled above) is the
+    // legal way to pass fewer arguments than parameters; anything else here is
+    // a malformed call, so reject it rather than silently accepting it. The
+    // caller records the returned message as a preprocessor error.
+    return absl::InvalidArgumentError(absl::StrCat(
+        "unexpected token while scanning arguments of macro call `",
+        macro_name_str,
+        ": expected ',' or ')', but got: ", (*token_iter)->ToString()));
   }
   if (parameters_size > 0) {
     while (parameters_size--) {
@@ -307,8 +311,15 @@
 
   if (config_.expand_macros) {
     verible::MacroCall macro_call;
-    RETURN_IF_ERROR(
-        ConsumeAndParseMacroCall(iter, generator, &macro_call, *found));
+    const absl::Status call_status =
+        ConsumeAndParseMacroCall(iter, generator, &macro_call, *found);
+    if (!call_status.ok()) {
+      // Surface a malformed macro call (e.g. an unterminated argument list) as
+      // a preprocessor error at the call site instead of silently accepting it.
+      std::string message(call_status.message());
+      preprocess_data_.errors.emplace_back(**iter, message);
+      return call_status;
+    }
     RETURN_IF_ERROR(ExpandMacro(macro_call, found));
   }
   auto &lexed = preprocess_data_.lexed_macros_backup.back();

verible/verilog/preprocessor/verilog-preprocess_test.cc

diff --git a/verible/verilog/preprocessor/verilog-preprocess_test.cc b/verible/verilog/preprocessor/verilog-preprocess_test.cc
index 7048888..64817c7 100644
--- a/verible/verilog/preprocessor/verilog-preprocess_test.cc
+++ b/verible/verilog/preprocessor/verilog-preprocess_test.cc
@@ -139,14 +139,15 @@
   }
 }
 
-// A function-like macro invoked with fewer arguments than parameters and no
-// closing ')' (EOF reached mid-call) must not spin ConsumeAndParseMacroCall
-// forever. Reaching any assertion below proves the preprocessor terminated and
-// back-filled the missing arguments instead of hanging.
-TEST(VerilogPreprocessTest, UnterminatedMacroCallDoesNotHang) {
+// A function-like macro invoked with no closing ')' (EOF reached mid-call) must
+// not spin ConsumeAndParseMacroCall forever. It is a malformed call, so the
+// preprocessor rejects it with an error rather than silently accepting it.
+// (Completing the test at all also proves the scan terminated instead of
+// hanging.)
+TEST(VerilogPreprocessTest, UnterminatedMacroCallIsRejected) {
   PreprocessorTester tester("`define FOO(a, b) a\n`FOO(x",
                             VerilogPreprocess::Config({.expand_macros = true}));
-  SUCCEED();
+  EXPECT_FALSE(tester.PreprocessorData().errors.empty());
 }
 
 #define EXPECT_PARSE_OK()                                                \