[clang] 6cef325 - [clang-format] Don't squash Verilog escaped identifiers

via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 26 15:51:43 PDT 2023


Author: sstwcw
Date: 2023-03-26T22:45:44Z
New Revision: 6cef325481a8efc039ae9df5630609fd3a84560c

URL: https://github.com/llvm/llvm-project/commit/6cef325481a8efc039ae9df5630609fd3a84560c
DIFF: https://github.com/llvm/llvm-project/commit/6cef325481a8efc039ae9df5630609fd3a84560c.diff

LOG: [clang-format] Don't squash Verilog escaped identifiers

An escaped identifier always needs a space following it so the parser
can tell it apart from the next token.

The unit tests are changed to use `FormatTestBase.h` because we need the
2-argument version of `verifyFormat`.  We also added the `messUp`
virtual function because Verilog needs a different version of it.

Reviewed By: HazardyKnusperkeks

Differential Revision: https://reviews.llvm.org/D146401

Added: 
    

Modified: 
    clang/lib/Format/TokenAnnotator.cpp
    clang/unittests/Format/FormatTestBase.h
    clang/unittests/Format/FormatTestVerilog.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 91f9d5719b5a..5da5dc96032a 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4348,6 +4348,11 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
       return true;
     }
   } else if (Style.isVerilog()) {
+    // An escaped identifier ends with whitespace.
+    if (Style.isVerilog() && Left.is(tok::identifier) &&
+        Left.TokenText[0] == '\\') {
+      return true;
+    }
     // Add space between things in a primitive's state table unless in a
     // transition like `(0?)`.
     if ((Left.is(TT_VerilogTableItem) &&

diff  --git a/clang/unittests/Format/FormatTestBase.h b/clang/unittests/Format/FormatTestBase.h
index ea214468965a..7d34bcc2d835 100644
--- a/clang/unittests/Format/FormatTestBase.h
+++ b/clang/unittests/Format/FormatTestBase.h
@@ -31,6 +31,10 @@ class FormatTestBase : public ::testing::Test {
 
   virtual FormatStyle getDefaultStyle() const { return getLLVMStyle(); }
 
+  virtual std::string messUp(llvm::StringRef Code) const {
+    return test::messUp(Code);
+  }
+
   std::string format(llvm::StringRef Code,
                      const std::optional<FormatStyle> &Style = {},
                      StatusCheck CheckComplete = SC_ExpectComplete,
@@ -105,8 +109,7 @@ class FormatTestBase : public ::testing::Test {
   void _verifyIncompleteFormat(const char *File, int Line, llvm::StringRef Code,
                                const std::optional<FormatStyle> &Style = {}) {
     testing::ScopedTrace t(File, Line, ::testing::Message() << Code.str());
-    EXPECT_EQ(Code.str(),
-              format(test::messUp(Code), Style, SC_ExpectIncomplete));
+    EXPECT_EQ(Code.str(), format(messUp(Code), Style, SC_ExpectIncomplete));
   }
 
   void
@@ -139,4 +142,4 @@ class FormatTestBase : public ::testing::Test {
 } // namespace format
 } // namespace clang
 
-#endif
\ No newline at end of file
+#endif

diff  --git a/clang/unittests/Format/FormatTestVerilog.cpp b/clang/unittests/Format/FormatTestVerilog.cpp
index 3a03525c2db7..88ce08207c95 100644
--- a/clang/unittests/Format/FormatTestVerilog.cpp
+++ b/clang/unittests/Format/FormatTestVerilog.cpp
@@ -6,47 +6,26 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "FormatTestUtils.h"
-#include "clang/Format/Format.h"
-#include "llvm/Support/Debug.h"
-#include "gtest/gtest.h"
+#include "FormatTestBase.h"
 
 #define DEBUG_TYPE "format-test"
 
 namespace clang {
 namespace format {
-
-class FormatTestVerilog : public ::testing::Test {
+namespace test {
+namespace {
+class FormatTestVerilog : public test::FormatTestBase {
 protected:
-  static std::string format(llvm::StringRef Code, unsigned Offset,
-                            unsigned Length, const FormatStyle &Style) {
-    LLVM_DEBUG(llvm::errs() << "---\n");
-    LLVM_DEBUG(llvm::errs() << Code << "\n\n");
-    std::vector<tooling::Range> Ranges(1, tooling::Range(Offset, Length));
-    tooling::Replacements Replaces = reformat(Style, Code, Ranges);
-    auto Result = applyAllReplacements(Code, Replaces);
-    EXPECT_TRUE(static_cast<bool>(Result));
-    LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
-    return *Result;
-  }
-
-  static std::string
-  format(llvm::StringRef Code,
-         const FormatStyle &Style = getLLVMStyle(FormatStyle::LK_Verilog)) {
-    return format(Code, 0, Code.size(), Style);
+  FormatStyle getDefaultStyle() const override {
+    return getLLVMStyle(FormatStyle::LK_Verilog);
   }
-
-  static void verifyFormat(
-      llvm::StringRef Code,
-      const FormatStyle &Style = getLLVMStyle(FormatStyle::LK_Verilog)) {
-    EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
-    EXPECT_EQ(Code.str(),
-              format(test::messUp(Code, /*HandleHash=*/false), Style));
+  std::string messUp(llvm::StringRef Code) const override {
+    return test::messUp(Code, /*HandleHash=*/false);
   }
 };
 
 TEST_F(FormatTestVerilog, Align) {
-  FormatStyle Style = getLLVMStyle(FormatStyle::LK_Verilog);
+  FormatStyle Style = getDefaultStyle();
   Style.AlignConsecutiveAssignments.Enabled = true;
   verifyFormat("x            <= x;\n"
                "sfdbddfbdfbb <= x;\n"
@@ -242,7 +221,7 @@ TEST_F(FormatTestVerilog, Case) {
                "    instruction3(ir);\n"
                "endcase");
   // Test indention options.
-  auto Style = getLLVMStyle(FormatStyle::LK_Verilog);
+  auto Style = getDefaultStyle();
   Style.IndentCaseLabels = false;
   verifyFormat("case (data)\n"
                "16'd0:\n"
@@ -268,7 +247,7 @@ TEST_F(FormatTestVerilog, Case) {
                "endcase",
                Style);
   // Other colons should not be mistaken as case colons.
-  Style = getLLVMStyle(FormatStyle::LK_Verilog);
+  Style = getDefaultStyle();
   Style.BitFieldColonSpacing = FormatStyle::BFCS_None;
   verifyFormat("case (x[1:0])\n"
                "endcase",
@@ -283,7 +262,7 @@ TEST_F(FormatTestVerilog, Case) {
   verifyFormat("default:\n"
                "  x[1 : 0] = x[1 : 0];",
                Style);
-  Style = getLLVMStyle(FormatStyle::LK_Verilog);
+  Style = getDefaultStyle();
   Style.SpacesInContainerLiterals = true;
   verifyFormat("case ('{x : x, default : 9})\n"
                "endcase",
@@ -355,8 +334,8 @@ TEST_F(FormatTestVerilog, Delay) {
   verifyFormat("#1.5s;");
   // The following expression should be on the same line.
   verifyFormat("#1 x = x;");
-  EXPECT_EQ("#1 x = x;", format("#1\n"
-                                "x = x;"));
+  verifyFormat("#1 x = x;", "#1\n"
+                            "x = x;");
 }
 
 TEST_F(FormatTestVerilog, Headers) {
@@ -486,7 +465,7 @@ TEST_F(FormatTestVerilog, Headers) {
                "                 b);\n"
                "endmodule");
   // With a concatenation in the names.
-  auto Style = getLLVMStyle(FormatStyle::LK_Verilog);
+  auto Style = getDefaultStyle();
   Style.ColumnLimit = 40;
   verifyFormat("`define X(x)                           \\\n"
                "  module test                          \\\n"
@@ -577,6 +556,28 @@ TEST_F(FormatTestVerilog, Hierarchy) {
                "endfunction : x");
 }
 
+TEST_F(FormatTestVerilog, Identifiers) {
+  // Escaped identifiers should not be split.
+  verifyFormat("\\busa+index");
+  verifyFormat("\\-clock");
+  verifyFormat("\\***error-condition***");
+  verifyFormat("\\net1\\/net2");
+  verifyFormat("\\{a,b}");
+  verifyFormat("\\a*(b+c)");
+  // Escaped identifiers can't be joined with the next token.  Extra space
+  // should be removed.
+  verifyFormat("\\busa+index ;", "\\busa+index\n"
+                                 ";");
+  verifyFormat("\\busa+index ;", "\\busa+index\r\n"
+                                 ";");
+  verifyFormat("\\busa+index ;", "\\busa+index  ;");
+  verifyFormat("\\busa+index ;", "\\busa+index\n"
+                                 " ;");
+  verifyFormat("\\busa+index ;");
+  verifyFormat("(\\busa+index );");
+  verifyFormat("\\busa+index \\busa+index ;");
+}
+
 TEST_F(FormatTestVerilog, If) {
   verifyFormat("if (x)\n"
                "  x = x;");
@@ -719,9 +720,9 @@ TEST_F(FormatTestVerilog, Operators) {
   verifyFormat("x <= x;");
 
   // Test that space is added between operators.
-  EXPECT_EQ("x = x < -x;", format("x=x<-x;"));
-  EXPECT_EQ("x = x << -x;", format("x=x<<-x;"));
-  EXPECT_EQ("x = x <<< -x;", format("x=x<<<-x;"));
+  verifyFormat("x = x < -x;", "x=x<-x;");
+  verifyFormat("x = x << -x;", "x=x<<-x;");
+  verifyFormat("x = x <<< -x;", "x=x<<<-x;");
 
   // Test that operators that are C++ identifiers get treated as operators.
   verifyFormat("solve s before d;");                       // before
@@ -732,41 +733,41 @@ TEST_F(FormatTestVerilog, Operators) {
 }
 
 TEST_F(FormatTestVerilog, Preprocessor) {
-  auto Style = getLLVMStyle(FormatStyle::LK_Verilog);
+  auto Style = getDefaultStyle();
   Style.ColumnLimit = 20;
 
   // Macro definitions.
-  EXPECT_EQ("`define X          \\\n"
-            "  if (x)           \\\n"
-            "    x = x;",
-            format("`define X if(x)x=x;", Style));
-  EXPECT_EQ("`define X(x)       \\\n"
-            "  if (x)           \\\n"
-            "    x = x;",
-            format("`define X(x) if(x)x=x;", Style));
-  EXPECT_EQ("`define X          \\\n"
-            "  x = x;           \\\n"
-            "  x = x;",
-            format("`define X x=x;x=x;", Style));
+  verifyFormat("`define X          \\\n"
+               "  if (x)           \\\n"
+               "    x = x;",
+               "`define X if(x)x=x;", Style);
+  verifyFormat("`define X(x)       \\\n"
+               "  if (x)           \\\n"
+               "    x = x;",
+               "`define X(x) if(x)x=x;", Style);
+  verifyFormat("`define X          \\\n"
+               "  x = x;           \\\n"
+               "  x = x;",
+               "`define X x=x;x=x;", Style);
   // Macro definitions with invocations inside.
-  EXPECT_EQ("`define LIST       \\\n"
-            "  `ENTRY           \\\n"
-            "  `ENTRY",
-            format("`define LIST \\\n"
-                   "`ENTRY \\\n"
-                   "`ENTRY",
-                   Style));
-  EXPECT_EQ("`define LIST       \\\n"
-            "  `x = `x;         \\\n"
-            "  `x = `x;",
-            format("`define LIST \\\n"
-                   "`x = `x; \\\n"
-                   "`x = `x;",
-                   Style));
-  EXPECT_EQ("`define LIST       \\\n"
-            "  `x = `x;         \\\n"
-            "  `x = `x;",
-            format("`define LIST `x=`x;`x=`x;", Style));
+  verifyFormat("`define LIST       \\\n"
+               "  `ENTRY           \\\n"
+               "  `ENTRY",
+               "`define LIST \\\n"
+               "`ENTRY \\\n"
+               "`ENTRY",
+               Style);
+  verifyFormat("`define LIST       \\\n"
+               "  `x = `x;         \\\n"
+               "  `x = `x;",
+               "`define LIST \\\n"
+               "`x = `x; \\\n"
+               "`x = `x;",
+               Style);
+  verifyFormat("`define LIST       \\\n"
+               "  `x = `x;         \\\n"
+               "  `x = `x;",
+               "`define LIST `x=`x;`x=`x;", Style);
   // Macro invocations.
   verifyFormat("`x = (`x1 + `x2 + x);");
   // Lines starting with a preprocessor directive should not be indented.
@@ -793,49 +794,49 @@ TEST_F(FormatTestVerilog, Preprocessor) {
       "undefineall",
   };
   for (auto &Name : Directives) {
-    EXPECT_EQ("if (x)\n"
-              "`" +
-                  Name +
-                  "\n"
-                  "  ;",
-              format("if (x)\n"
-                     "`" +
-                         Name +
-                         "\n"
-                         ";",
-                     Style));
+    verifyFormat("if (x)\n"
+                 "`" +
+                     Name +
+                     "\n"
+                     "  ;",
+                 "if (x)\n"
+                 "`" +
+                     Name +
+                     "\n"
+                     ";",
+                 Style);
   }
   // Lines starting with a regular macro invocation should be indented as a
   // normal line.
-  EXPECT_EQ("if (x)\n"
-            "  `x = `x;\n"
-            "`timescale 1ns / 1ps",
-            format("if (x)\n"
-                   "`x = `x;\n"
-                   "`timescale 1ns / 1ps",
-                   Style));
-  EXPECT_EQ("if (x)\n"
-            "`timescale 1ns / 1ps\n"
-            "  `x = `x;",
-            format("if (x)\n"
-                   "`timescale 1ns / 1ps\n"
-                   "`x = `x;",
-                   Style));
+  verifyFormat("if (x)\n"
+               "  `x = `x;\n"
+               "`timescale 1ns / 1ps",
+               "if (x)\n"
+               "`x = `x;\n"
+               "`timescale 1ns / 1ps",
+               Style);
+  verifyFormat("if (x)\n"
+               "`timescale 1ns / 1ps\n"
+               "  `x = `x;",
+               "if (x)\n"
+               "`timescale 1ns / 1ps\n"
+               "`x = `x;",
+               Style);
   std::string NonDirectives[] = {
       // For `__FILE__` and `__LINE__`, although the standard classifies them as
       // preprocessor directives, they are used like regular macros.
       "__FILE__", "__LINE__", "elif", "foo", "x",
   };
   for (auto &Name : NonDirectives) {
-    EXPECT_EQ("if (x)\n"
-              "  `" +
-                  Name + ";",
-              format("if (x)\n"
-                     "`" +
-                         Name +
-                         "\n"
-                         ";",
-                     Style));
+    verifyFormat("if (x)\n"
+                 "  `" +
+                     Name + ";",
+                 "if (x)\n"
+                 "`" +
+                     Name +
+                     "\n"
+                     ";",
+                 Style);
   }
 }
 
@@ -945,5 +946,7 @@ TEST_F(FormatTestVerilog, StructuredProcedure) {
   verifyFormat("forever\n"
                "  x <= x;");
 }
+} // namespace
+} // namespace test
 } // namespace format
-} // end namespace clang
+} // namespace clang


        


More information about the cfe-commits mailing list