Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-format] Add space in Verilog tagged unions #71354

Merged
merged 2 commits into from
Dec 2, 2023

Conversation

sstwcw
Copy link
Contributor

@sstwcw sstwcw commented Nov 6, 2023

In a tagged union expression, there should be a space between the field name and the data. Previously, the tag could be recognized as part of a dotted identifier or a struct literal, and the space would be omitted.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 6, 2023

@llvm/pr-subscribers-clang-format

Author: None (sstwcw)

Changes

In a tagged union expression, there should be a space between the field name and the data. Previously, the tag could be recognized as part of a dotted identifier or a struct literal, and the space would be omitted.


Full diff: https://github.com/llvm/llvm-project/pull/71354.diff

2 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+8-1)
  • (modified) clang/unittests/Format/FormatTestVerilog.cpp (+12-2)
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 729e7e370bf62ea..263125d5711a95b 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4694,8 +4694,15 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
         Left.isOneOf(TT_VerilogDimensionedTypeName, Keywords.kw_function)) {
       return true;
     }
+    // In a tagged union expression, there should be a space after the tag.
+    if (Right.isOneOf(tok::period, Keywords.kw_apostrophe) &&
+        Keywords.isVerilogIdentifier(Left) && Left.getPreviousNonComment() &&
+        Left.getPreviousNonComment()->is(Keywords.kw_tagged)) {
+      return true;
+    }
     // Don't add spaces between a casting type and the quote or repetition count
-    // and the brace.
+    // and the brace.  The case of tagged union expressions is handled by the
+    // previous rule.
     if ((Right.is(Keywords.kw_apostrophe) ||
          (Right.is(BK_BracedInit) && Right.is(tok::l_brace))) &&
         !(Left.isOneOf(Keywords.kw_assign, Keywords.kw_unique) ||
diff --git a/clang/unittests/Format/FormatTestVerilog.cpp b/clang/unittests/Format/FormatTestVerilog.cpp
index 1c2692467987d9b..99d0fcca38c3993 100644
--- a/clang/unittests/Format/FormatTestVerilog.cpp
+++ b/clang/unittests/Format/FormatTestVerilog.cpp
@@ -359,6 +359,11 @@ TEST_F(FormatTestVerilog, Case) {
                "      arg);\n"
                "endcase",
                Style);
+
+  verifyFormat("case (v) matches\n"
+               "  tagged Valid .n:\n"
+               "    ;\n"
+               "endcase");
 }
 
 TEST_F(FormatTestVerilog, Coverage) {
@@ -1278,12 +1283,17 @@ TEST_F(FormatTestVerilog, StructLiteral) {
   verifyFormat("c = '{'{1, 1.0}, '{2, 2.0}};");
   verifyFormat("c = '{a: 0, b: 0.0};");
   verifyFormat("c = '{a: 0, b: 0.0, default: 0};");
+  verifyFormat("d = {int: 1, shortreal: 1.0};");
+  verifyFormat("c = '{default: 0};");
+
+  // The identifier before the quote can be either a tag or a type case.  There
+  // should be a space between the tag and the quote.
   verifyFormat("c = ab'{a: 0, b: 0.0};");
   verifyFormat("c = ab'{cd: cd'{1, 1.0}, ef: ef'{2, 2.0}};");
   verifyFormat("c = ab'{cd'{1, 1.0}, ef'{2, 2.0}};");
-  verifyFormat("d = {int: 1, shortreal: 1.0};");
   verifyFormat("d = ab'{int: 1, shortreal: 1.0};");
-  verifyFormat("c = '{default: 0};");
+  verifyFormat("x = tagged Add '{e1, 4, ed};");
+
   auto Style = getDefaultStyle();
   Style.SpacesInContainerLiterals = true;
   verifyFormat("c = '{a : 0, b : 0.0};", Style);

// Don't add spaces between a casting type and the quote or repetition count
// and the brace.
// and the brace. The case of tagged union expressions is handled by the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// and the brace. The case of tagged union expressions is handled by the
// and the brace. The case of tagged union expressions is handled by the

clang/lib/Format/TokenAnnotator.cpp Show resolved Hide resolved
In a tagged union expression, there should be a space between the field
name and the data.  Previously, the tag could be recognized as part of a
dotted identifier or a struct literal, and the space would be omitted.
@sstwcw sstwcw merged commit b3e80d8 into llvm:main Dec 2, 2023
3 checks passed
@sstwcw sstwcw deleted the format-verilog-tagged branch December 2, 2023 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants