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] Indent Verilog case statements with comments #71353

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

sstwcw
Copy link
Contributor

@sstwcw sstwcw commented Nov 6, 2023

If a line contains a comment outside of (fake) parentheses, the part following it is indented according to CurrentState.Indent. A Verilog case label and the statement that follows are broken with mustBreakBefore. So the part that follows the case label needs some special handling. Previously, that variable was left out. So the indentation was wrong when there was a comment.

old:

case (data)
  16'd0:
    result = //
        10'b0111111111;
endcase
case (data)
  16'd0:
    //

  //
  result = //
  10'b0111111111;
endcase

new:

case (data)
  16'd0:
    result = //
        10'b0111111111;
endcase
case (data)
  16'd0:
    //

    //
    result = //
        10'b0111111111;
endcase

If a line contains a comment outside of (fake) parentheses, the part
following it is indented according to `CurrentState.Indent`.  A Verilog
case label and the statement that follows are broken with
mustBreakBefore.  So the part that follows the case label needs some
special handling.  Previously, that variable was left out.  So the
indentation was wrong when there was a comment.

old:

```Verilog
case (data)
  16'd0:
    result = //
        10'b0111111111;
endcase
case (data)
  16'd0:
    //

  //
  result = //
  10'b0111111111;
endcase
```

new:

```Verilog
case (data)
  16'd0:
    result = //
        10'b0111111111;
endcase
case (data)
  16'd0:
    //

    //
    result = //
        10'b0111111111;
endcase
```
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 6, 2023

@llvm/pr-subscribers-clang-format

Author: None (sstwcw)

Changes

If a line contains a comment outside of (fake) parentheses, the part following it is indented according to CurrentState.Indent. A Verilog case label and the statement that follows are broken with mustBreakBefore. So the part that follows the case label needs some special handling. Previously, that variable was left out. So the indentation was wrong when there was a comment.

old:

case (data)
  16'd0:
    result = //
        10'b0111111111;
endcase
case (data)
  16'd0:
    //

  //
  result = //
  10'b0111111111;
endcase

new:

case (data)
  16'd0:
    result = //
        10'b0111111111;
endcase
case (data)
  16'd0:
    //

    //
    result = //
        10'b0111111111;
endcase

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

2 Files Affected:

  • (modified) clang/lib/Format/ContinuationIndenter.cpp (+4-1)
  • (modified) clang/unittests/Format/FormatTestVerilog.cpp (+14)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 3a829cdedb77fc7..17a039abb30f8da 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1208,8 +1208,10 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
 
   // Indentation of the statement following a Verilog case label is taken care
   // of in moveStateToNextToken.
-  if (Style.isVerilog() && Keywords.isVerilogEndOfLabel(Previous))
+  if (Style.isVerilog() && PreviousNonComment &&
+      Keywords.isVerilogEndOfLabel(*PreviousNonComment)) {
     return State.FirstIndent;
+  }
 
   if (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths &&
       State.Line->First->is(tok::kw_enum)) {
@@ -1612,6 +1614,7 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
       State.NextToken->MustBreakBefore &&
       Keywords.isVerilogEndOfLabel(Current)) {
     State.FirstIndent += Style.IndentWidth;
+    CurrentState.Indent = State.FirstIndent;
   }
 
   unsigned Penalty =
diff --git a/clang/unittests/Format/FormatTestVerilog.cpp b/clang/unittests/Format/FormatTestVerilog.cpp
index 1c2692467987d9b..9d5306c039ec334 100644
--- a/clang/unittests/Format/FormatTestVerilog.cpp
+++ b/clang/unittests/Format/FormatTestVerilog.cpp
@@ -344,6 +344,20 @@ TEST_F(FormatTestVerilog, Case) {
                "        longfunction( //\n"
                "            arg);\n"
                "endcase");
+  verifyFormat("case (data)\n"
+               "  16'd0:\n"
+               "    //\n"
+               "    result = //\n"
+               "        10'b0111111111;\n"
+               "endcase");
+  verifyFormat("case (data)\n"
+               "  16'd0:\n"
+               "    //\n"
+               "\n"
+               "    //\n"
+               "    result = //\n"
+               "        10'b0111111111;\n"
+               "endcase");
   Style = getDefaultStyle();
   Style.ContinuationIndentWidth = 1;
   verifyFormat("case (data)\n"

@sstwcw sstwcw merged commit 9fa2d74 into llvm:main Nov 29, 2023
4 checks passed
@sstwcw sstwcw deleted the format-verilog-comments branch November 29, 2023 15:19
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