Merge pull request #2545 from kbrunham-intel/fix/2544

Fix formatter lexical mismatch for macros at end of line

verible/verilog/analysis/verilog-equivalence.cc

diff --git a/verible/verilog/analysis/verilog-equivalence.cc b/verible/verilog/analysis/verilog-equivalence.cc
index f8a47d0..68c8c98 100644
--- a/verible/verilog/analysis/verilog-equivalence.cc
+++ b/verible/verilog/analysis/verilog-equivalence.cc
@@ -89,6 +89,20 @@
   return IsUnlexed(verilog_tokentype(token.token_enum()));
 }
 
+// MacroIdentifier vs MacroIdItem depends only on whether the macro ends the
+// line (see POST_MACRO_ID in verilog.lex). Spelling-equal macros are
+// format-equivalent across that reclassification.
+static bool AreSpellingEqualLineEndingMacros(const TokenInfo &left,
+                                             const TokenInfo &right) {
+  const auto is_line_ending_macro = [](int token_enum) {
+    return token_enum == verilog_tokentype::MacroIdentifier ||
+           token_enum == verilog_tokentype::MacroIdItem;
+  };
+  return is_line_ending_macro(left.token_enum()) &&
+         is_line_ending_macro(right.token_enum()) &&
+         left.text() == right.text();
+}
+
 DiffStatus VerilogLexicallyEquivalent(
     std::string_view left, std::string_view right,
     const std::function<bool(const verible::TokenInfo &)> &remove_predicate,
@@ -167,11 +181,17 @@
   DiffStatus diff_status = DiffStatus::kEquivalent;
   auto recursive_comparator = [&](const TokenSequence::const_iterator l,
                                   const TokenSequence::const_iterator r) {
+    // Some token enums differ only by surrounding whitespace (e.g. whether a
+    // macro or ')' ends a line). Treat those pairs as matching enums when the
+    // spelling is unchanged so FormatEquivalent tolerates re-wrapping.
+    const bool whitespace_dependent_macro_enum_match =
+        ((l->token_enum() == verilog_tokentype::MacroCallCloseToEndLine &&
+          r->text() == ")") ||
+         (r->token_enum() == verilog_tokentype::MacroCallCloseToEndLine &&
+          l->text() == ")") ||
+         AreSpellingEqualLineEndingMacros(*l, *r));
     if (l->token_enum() != r->token_enum() &&
-        !((l->token_enum() == verilog_tokentype::MacroCallCloseToEndLine &&
-           r->text() == ")") ||
-          (r->token_enum() == verilog_tokentype::MacroCallCloseToEndLine &&
-           l->text() == ")"))) {
+        !whitespace_dependent_macro_enum_match) {
       if (errstream != nullptr) {
         *errstream << "Mismatched token enums.  got: ";
         token_printer(*l, *errstream);
@@ -258,6 +278,9 @@
              (r.text() == ")"))) {
           return true;
         }
+        if (AreSpellingEqualLineEndingMacros(l, r)) {
+          return true;
+        }
         return l.EquivalentWithoutLocation(r);
       },
       errstream);

verible/verilog/analysis/verilog-equivalence_test.cc

diff --git a/verible/verilog/analysis/verilog-equivalence_test.cc b/verible/verilog/analysis/verilog-equivalence_test.cc
index 90b0ee1..1cb9436 100644
--- a/verible/verilog/analysis/verilog-equivalence_test.cc
+++ b/verible/verilog/analysis/verilog-equivalence_test.cc
@@ -255,6 +255,21 @@
   }
 }
 
+// MacroIdentifier vs MacroIdItem depends on whether the macro ends the line.
+TEST(FormatEquivalentTest, EquivalenceOfMacroIdentifierAndMacroIdItem) {
+  const char *kSameSpelling[] = {
+      "assign x = f(`TOKEN);\n",
+      "assign x = f(\n`TOKEN\n);\n",
+  };
+  ExpectCompareWithErrstream(FormatEquivalent, DiffStatus::kEquivalent,
+                             kSameSpelling[0], kSameSpelling[1]);
+
+  // Different macro names remain different.
+  ExpectCompareWithErrstream(FormatEquivalent, DiffStatus::kDifferent,
+                             "assign x = f(`TOKEN);\n",
+                             "assign x = f(`OTHER);\n");
+}
+
 TEST(FormatEquivalentTest, DiagnosticMismatch) {
   const char *kTestCases[] = {
       "module foo;\n",

verible/verilog/formatting/formatter_test.cc

diff --git a/verible/verilog/formatting/formatter_test.cc b/verible/verilog/formatting/formatter_test.cc
index bd0f775..13a9fb4 100644
--- a/verible/verilog/formatting/formatter_test.cc
+++ b/verible/verilog/formatting/formatter_test.cc
@@ -19456,6 +19456,24 @@
   }
 }
 
+// Regression for https://github.com/chipsalliance/verible/issues/2544:
+// Wrapping a $bits(...)'(...) cast may leave `MACRO at EOL, reclassifying
+// MacroIdentifier as MacroIdItem. FormatEquivalent must accept that, and
+// formatting must still pass verification.
+TEST(FormatterEndToEndTest, MacroBeforeCloseParenFormatEquivalent) {
+  static constexpr std::string_view kInput =
+      "module m;\n"
+      "  assign result_value = $bits(result_value)'( "
+      "compare_bytes(input_data[DATA_WIDTH_INT-1:0], "
+      "input_datak[STROBE_WIDTH_INT-1:0], `TOKEN_BYTE) );\n"
+      "endmodule\n";
+  FormatStyle style;
+  std::ostringstream stream;
+  const auto status = FormatVerilog(kInput, "<filename>", style, stream);
+  EXPECT_OK(status) << status.message();
+  EXPECT_THAT(stream.str(), testing::HasSubstr("`TOKEN_BYTE"));
+}
+
 // Regression for https://github.com/chipsalliance/verible/issues/2542:
 // Continuation EOL comments after a wrapped assign must keep a stable column
 // across re-format (convergence).