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

2023-03-26 Thread sstwcw via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6cef325481a8: [clang-format] Dont squash Verilog 
escaped identifiers (authored by sstwcw).

Changed prior to commit:
  https://reviews.llvm.org/D146401?vs=506454=508444#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146401/new/

https://reviews.llvm.org/D146401

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

Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- clang/unittests/Format/FormatTestVerilog.cpp
+++ 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 ) {
-LLVM_DEBUG(llvm::errs() << "---\n");
-LLVM_DEBUG(llvm::errs() << Code << "\n\n");
-std::vector Ranges(1, tooling::Range(Offset, Length));
-tooling::Replacements Replaces = reformat(Style, Code, Ranges);
-auto Result = applyAllReplacements(Code, Replaces);
-EXPECT_TRUE(static_cast(Result));
-LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
-return *Result;
-  }
-
-  static std::string
-  format(llvm::StringRef Code,
- const FormatStyle  = 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  = 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 @@
"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 @@
"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 @@
   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 @@
   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 @@
" 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 @@
"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"
+

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

2023-03-25 Thread Björn Schäpers via Phabricator via cfe-commits 
HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/unittests/Format/FormatTestVerilog.cpp:19
 protected:
-  static std::string format(llvm::StringRef Code, unsigned Offset,
-unsigned Length, const FormatStyle ) {
-LLVM_DEBUG(llvm::errs() << "---\n");
-LLVM_DEBUG(llvm::errs() << Code << "\n\n");
-std::vector Ranges(1, tooling::Range(Offset, Length));
-tooling::Replacements Replaces = reformat(Style, Code, Ranges);
-auto Result = applyAllReplacements(Code, Replaces);
-EXPECT_TRUE(static_cast(Result));
-LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
-return *Result;
-  }
-
-  static std::string
-  format(llvm::StringRef Code,
- const FormatStyle  = getLLVMStyle(FormatStyle::LK_Verilog)) {
-return format(Code, 0, Code.size(), Style);
+  virtual FormatStyle getDefaultStyle() const override {
+return getLLVMStyle(FormatStyle::LK_Verilog);

Drop that virtual.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146401/new/

https://reviews.llvm.org/D146401

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-03-22 Thread sstwcw via Phabricator via cfe-commits 
sstwcw marked an inline comment as done.
sstwcw added inline comments.



Comment at: clang/unittests/Format/FormatTestVerilog.cpp:897
 }
+} // namespace
+} // namespace test

MyDeveloperDay wrote:
> is this correct? do you have an anonymous namespace?
It is correct.  The class `FormatTestVerilog` is only used in this file.  Other 
files have it too.  I didn't notice it when I copied the first few lines from 
`FormatTest.cpp` this time, but I probably forgot to add it when creating the 
file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146401/new/

https://reviews.llvm.org/D146401

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-03-22 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTestVerilog.cpp:897
 }
+} // namespace
+} // namespace test

is this correct? do you have an anonymous namespace?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146401/new/

https://reviews.llvm.org/D146401

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2023-03-19 Thread sstwcw via Phabricator via cfe-commits 
sstwcw created this revision.
sstwcw added reviewers: HazardyKnusperkeks, MyDeveloperDay, owenpan, rymiel.
Herald added a project: All.
sstwcw requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146401

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

Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- clang/unittests/Format/FormatTestVerilog.cpp
+++ 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 ) {
-LLVM_DEBUG(llvm::errs() << "---\n");
-LLVM_DEBUG(llvm::errs() << Code << "\n\n");
-std::vector Ranges(1, tooling::Range(Offset, Length));
-tooling::Replacements Replaces = reformat(Style, Code, Ranges);
-auto Result = applyAllReplacements(Code, Replaces);
-EXPECT_TRUE(static_cast(Result));
-LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
-return *Result;
-  }
-
-  static std::string
-  format(llvm::StringRef Code,
- const FormatStyle  = getLLVMStyle(FormatStyle::LK_Verilog)) {
-return format(Code, 0, Code.size(), Style);
+  virtual FormatStyle getDefaultStyle() const override {
+return getLLVMStyle(FormatStyle::LK_Verilog);
   }
-
-  static void verifyFormat(
-  llvm::StringRef Code,
-  const FormatStyle  = 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));
+  virtual 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"
@@ -225,7 +204,7 @@
"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"
@@ -251,7 +230,7 @@
"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",
@@ -266,7 +245,7 @@
   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",
@@ -338,8 +317,8 @@
   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) {
@@ -469,7 +448,7 @@
" 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"
@@ -560,6 +539,28 @@
"endfunction : x");
 }
 
+TEST_F(FormatTestVerilog, Identifiers) {
+  // Escaped identifiers should not be split.
+  verifyFormat("\\busa+index");
+  verifyFormat("\\-clock");
+  verifyFormat("\\***error-condition***");
+  verifyFormat("\\net1\\/net2");
+