Merge branch 'chipsalliance:master' into fix/2352

verible/verilog/formatting/formatter_test.cc

diff --git a/verible/verilog/formatting/formatter_test.cc b/verible/verilog/formatting/formatter_test.cc
index aad3e60..894254f 100644
--- a/verible/verilog/formatting/formatter_test.cc
+++ b/verible/verilog/formatting/formatter_test.cc
@@ -128,6 +128,41 @@
   RunFormatterTestCases40(kSmokeFormatterTestCases);
 }
 
+// Regression for https://github.com/chipsalliance/verible/issues/2352:
+// '/' between identifiers in a macro argument is a path separator and must
+// not be spaced as a division operator (that breaks compiles).
+TEST(FormatterEndToEndTest, MacroArgPathSeparatorsKeepNoSpace) {
+  static constexpr FormatterTestCase kTestCases[] = {
+      {// Original issue sample
+       "`PROJECT_INCLUDE(`PATH_MY_MODULE/src/config_class.sv)\n",
+       "`PROJECT_INCLUDE(`PATH_MY_MODULE/src/config_class.sv)\n"},
+      {// Extra spaces around '/' are removed in macro args
+       "`PROJECT_INCLUDE(`PATH_MY_MODULE / src / config_class.sv)\n",
+       "`PROJECT_INCLUDE(`PATH_MY_MODULE/src/config_class.sv)\n"},
+      {// Nested directories
+       "`INCLUDE(foo/bar/baz.svh)\n", "`INCLUDE(foo/bar/baz.svh)\n"},
+      {// Path as a later argument
+       "`LOAD(cfg, `ROOT/hw/ip/file.sv)\n",
+       "`LOAD(cfg, `ROOT/hw/ip/file.sv)\n"},
+      {// Division between identifiers outside macros still gets spaces
+       "module m;\n"
+       "  assign x = a/b;\n"
+       "endmodule\n",
+       "module m;\n"
+       "  assign x = a / b;\n"
+       "endmodule\n"},
+  };
+  FormatStyle style;  // default column_limit (100)
+  for (const auto &test_case : kTestCases) {
+    VLOG(1) << "code-to-format:\n" << test_case.input << "<EOF>";
+    std::ostringstream stream;
+    const auto status =
+        FormatVerilog(test_case.input, "<filename>", style, stream);
+    EXPECT_OK(status) << status.message();
+    EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+  }
+}
+
 }  // namespace
 }  // namespace formatter
 }  // namespace verilog

verible/verilog/formatting/token-annotator.cc

diff --git a/verible/verilog/formatting/token-annotator.cc b/verible/verilog/formatting/token-annotator.cc
index ebaeb21..fa3aadd 100644
--- a/verible/verilog/formatting/token-annotator.cc
+++ b/verible/verilog/formatting/token-annotator.cc
@@ -120,6 +120,29 @@
       {});
 }
 
+// '/' between identifiers inside a macro argument is a filesystem path
+// (e.g. `INCLUDE(`PATH/src/file.sv)), not a division operator (issue #2352).
+static bool InMacroArgumentContext(const SyntaxTreeContext &context) {
+  return context.IsInsideFirst({NodeEnum::kMacroArgList, NodeEnum::kMacroCall,
+                                NodeEnum::kMacroGenericItem},
+                               {});
+}
+
+static bool IsPathSeparatorSlashInMacroArg(
+    const PreFormatToken &left, const PreFormatToken &right,
+    const SyntaxTreeContext &left_context,
+    const SyntaxTreeContext &right_context) {
+  if (left.TokenEnum() != '/' && right.TokenEnum() != '/') return false;
+  const bool identifier_on_other_side =
+      (left.TokenEnum() == '/' &&
+       right.format_token_enum == FormatTokenType::identifier) ||
+      (right.TokenEnum() == '/' &&
+       left.format_token_enum == FormatTokenType::identifier);
+  if (!identifier_on_other_side) return false;
+  return InMacroArgumentContext(left_context) ||
+         InMacroArgumentContext(right_context);
+}
+
 static bool IsAnySemicolon(const PreFormatToken &ftoken) {
   // These are just syntactically disambiguated versions of ';'.
   return ftoken.TokenEnum() == ';' ||
@@ -228,6 +251,10 @@
   // Consider assignment operators in the same class as binary operators.
   if (left.format_token_enum == FormatTokenType::binary_operator ||
       right.format_token_enum == FormatTokenType::binary_operator) {
+    if (IsPathSeparatorSlashInMacroArg(left, right, left_context,
+                                       right_context)) {
+      return {0, "No space around '/' path separators in macro arguments"};
+    }
     // Inside [], allows 0 or 1 spaces, and symmetrize.
     // TODO(fangism): make this behavior configurable
     if (right.format_token_enum == FormatTokenType::binary_operator &&

verible/verilog/formatting/token-annotator_test.cc

diff --git a/verible/verilog/formatting/token-annotator_test.cc b/verible/verilog/formatting/token-annotator_test.cc
index 04c9832..d2df9c9 100644
--- a/verible/verilog/formatting/token-annotator_test.cc
+++ b/verible/verilog/formatting/token-annotator_test.cc
@@ -4715,6 +4715,48 @@
           {/* unspecified context */},
           {1, SpacingOptions::kUndecided},
       },
+      // '/' between identifiers is a path separator in macro args (#2352),
+      // but remains a binary operator in other contexts.
+      {
+          DefaultStyle,
+          {verilog_tokentype::MacroIdentifier, "`PATH"},
+          {'/', "/"},
+          {NodeEnum::kMacroArgList, NodeEnum::kMacroCall},
+          {NodeEnum::kMacroArgList, NodeEnum::kMacroCall},
+          {0, SpacingOptions::kUndecided},
+      },
+      {
+          DefaultStyle,
+          {'/', "/"},
+          {verilog_tokentype::SymbolIdentifier, "src"},
+          {NodeEnum::kMacroArgList, NodeEnum::kMacroCall},
+          {NodeEnum::kMacroArgList, NodeEnum::kMacroCall},
+          {0, SpacingOptions::kUndecided},
+      },
+      {
+          DefaultStyle,
+          {verilog_tokentype::SymbolIdentifier, "src"},
+          {'/', "/"},
+          {NodeEnum::kMacroArgList, NodeEnum::kMacroCall},
+          {NodeEnum::kMacroArgList, NodeEnum::kMacroCall},
+          {0, SpacingOptions::kUndecided},
+      },
+      {
+          DefaultStyle,
+          {verilog_tokentype::SymbolIdentifier, "a"},
+          {'/', "/"},
+          {/* expression, not a macro argument */},
+          {/* expression, not a macro argument */},
+          {1, SpacingOptions::kUndecided},
+      },
+      {
+          DefaultStyle,
+          {'/', "/"},
+          {verilog_tokentype::SymbolIdentifier, "b"},
+          {/* expression, not a macro argument */},
+          {/* expression, not a macro argument */},
+          {1, SpacingOptions::kUndecided},
+      },
   };
   int test_index = 0;
   for (const auto &test_case : kTestCases) {