[clang] 369e876 - [clang-format] Stop comment disrupting indentation of Verilog ports

via cfe-commits cfe-commits at lists.llvm.org
Mon May 15 19:58:15 PDT 2023


Author: sstwcw
Date: 2023-05-16T02:56:58Z
New Revision: 369e8762b4d658e6e8f8310200aadf043bd7f01e

URL: https://github.com/llvm/llvm-project/commit/369e8762b4d658e6e8f8310200aadf043bd7f01e
DIFF: https://github.com/llvm/llvm-project/commit/369e8762b4d658e6e8f8310200aadf043bd7f01e.diff

LOG: [clang-format] Stop comment disrupting indentation of Verilog ports

Before:

```
module x
    #( //
        parameter x)
    ( //
        input y);
endmodule
```

After:

```
module x
    #(//
      parameter x)
    (//
     input y);
endmodule
```

If the first line in a port or parameter list is not a comment, the
following lines will be aligned to the first line as intended:

```
module x
    #(parameter x1,
      parameter x2)
    (input y,
     input y2);
endmodule
```

Previously, the indentation would be changed to an extra continuation
indentation relative to the start of the parenthesis or the hash if
the first token inside the parentheses was a comment.  It is a feature
introduced in ddaa9be97839.  The feature enabled one to insert a `//`
comment right after an opening parentheses to put the function
arguments on a new line with a small indentation regardless of how
long the function name is, like this:

```
someFunction(anotherFunction( // Force break.
    parameter));
```

People are unlikely to use this feature in a Verilog port list because
the formatter already puts the port list on its own lines.  A comment
at the start of a port list is probably a comment for the port on the
next line.

We also removed the space before the comment so that its indentation
would be same as that for a line comment anywhere else in the port
list.

Reviewed By: HazardyKnusperkeks

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

Added: 
    

Modified: 
    clang/docs/ClangFormatStyleOptions.rst
    clang/include/clang/Format/Format.h
    clang/lib/Format/ContinuationIndenter.cpp
    clang/lib/Format/FormatToken.h
    clang/lib/Format/TokenAnnotator.cpp
    clang/lib/Format/UnwrappedLineParser.cpp
    clang/unittests/Format/FormatTestVerilog.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index ce4c39694e0c5..56b6b62e3138c 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -5069,9 +5069,12 @@ the configuration (without a prefix: ``Auto``).
   The number of spaces before trailing line comments
   (``//`` - comments).
 
-  This does not affect trailing block comments (``/*`` - comments) as
-  those commonly have 
diff erent usage patterns and a number of special
-  cases.
+  This does not affect trailing block comments (``/*`` - comments) as those
+  commonly have 
diff erent usage patterns and a number of special cases.  In
+  the case of Verilog, it doesn't affect a comment right after the opening
+  parenthesis in the port or parameter list in a module header, because it
+  is probably for the port on the following line instead of the parenthesis
+  it follows.
 
   .. code-block:: c++
 

diff  --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 40f1ae0b49d46..1cc1037562b89 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -4019,9 +4019,12 @@ struct FormatStyle {
   /// The number of spaces before trailing line comments
   /// (``//`` - comments).
   ///
-  /// This does not affect trailing block comments (``/*`` - comments) as
-  /// those commonly have 
diff erent usage patterns and a number of special
-  /// cases.
+  /// This does not affect trailing block comments (``/*`` - comments) as those
+  /// commonly have 
diff erent usage patterns and a number of special cases.  In
+  /// the case of Verilog, it doesn't affect a comment right after the opening
+  /// parenthesis in the port or parameter list in a module header, because it
+  /// is probably for the port on the following line instead of the parenthesis
+  /// it follows.
   /// \code
   ///    SpacesBeforeTrailingComments: 3
   ///    void f() {

diff  --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 3ae9d1997880f..f6f6bf61f1c37 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -748,7 +748,8 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
       !CurrentState.IsCSharpGenericTypeConstraint && Previous.opensScope() &&
       Previous.isNot(TT_ObjCMethodExpr) && Previous.isNot(TT_RequiresClause) &&
       !(Current.MacroParent && Previous.MacroParent) &&
-      (Current.isNot(TT_LineComment) || Previous.is(BK_BracedInit))) {
+      (Current.isNot(TT_LineComment) ||
+       Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen))) {
     CurrentState.Indent = State.Column + Spaces;
     CurrentState.IsAligned = true;
   }

diff  --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 1a4ff4d53925f..dd23bd35411dc 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -156,6 +156,9 @@ namespace format {
   /* list of port connections or parameters in a module instantiation */       \
   TYPE(VerilogInstancePortComma)                                               \
   TYPE(VerilogInstancePortLParen)                                              \
+  /* A parenthesized list within which line breaks are inserted by the         \
+   * formatter, for example the list of ports in a module header. */           \
+  TYPE(VerilogMultiLineListLParen)                                             \
   /* for the base in a number literal, not including the quote */              \
   TYPE(VerilogNumberBase)                                                      \
   /* like `(strong1, pull0)` */                                                \

diff  --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index be734858b84ee..fe32fa1013567 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3312,6 +3312,8 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const {
       if (Prev->is(BK_BracedInit) && Prev->opensScope()) {
         Current->SpacesRequiredBefore =
             (Style.Cpp11BracedListStyle && !Style.SpacesInParentheses) ? 0 : 1;
+      } else if (Prev->is(TT_VerilogMultiLineListLParen)) {
+        Current->SpacesRequiredBefore = 0;
       } else {
         Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
       }

diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 64035b0976f55..31b45fa7cc898 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -4196,11 +4196,14 @@ unsigned UnwrappedLineParser::parseVerilogHierarchyHeader() {
     if (FormatTok->is(Keywords.kw_verilogHash)) {
       NewLine();
       nextToken();
-      if (FormatTok->is(tok::l_paren))
+      if (FormatTok->is(tok::l_paren)) {
+        FormatTok->setFinalizedType(TT_VerilogMultiLineListLParen);
         parseParens();
+      }
     }
     if (FormatTok->is(tok::l_paren)) {
       NewLine();
+      FormatTok->setFinalizedType(TT_VerilogMultiLineListLParen);
       parseParens();
     }
 

diff  --git a/clang/unittests/Format/FormatTestVerilog.cpp b/clang/unittests/Format/FormatTestVerilog.cpp
index e2c9faff1fd5a..5c94d5afbac96 100644
--- a/clang/unittests/Format/FormatTestVerilog.cpp
+++ b/clang/unittests/Format/FormatTestVerilog.cpp
@@ -517,6 +517,15 @@ TEST_F(FormatTestVerilog, Headers) {
                "    (input var x `a, //\n"
                "                 b);\n"
                "endmodule");
+  // A line comment shouldn't disrupt the indentation of the port list.
+  verifyFormat("extern module x\n"
+               "    (//\n"
+               "     output y);");
+  verifyFormat("extern module x\n"
+               "    #(//\n"
+               "      parameter x)\n"
+               "    (//\n"
+               "     output y);");
   // With a concatenation in the names.
   auto Style = getDefaultStyle();
   Style.ColumnLimit = 40;


        


More information about the cfe-commits mailing list