https://github.com/sstwcw updated https://github.com/llvm/llvm-project/pull/65861
>From c1982d451e6ca8e3cf1269677aabe984000f6801 Mon Sep 17 00:00:00 2001 From: sstwcw <su3e8a96kzl...@posteo.net> Date: Sat, 9 Sep 2023 20:35:33 +0000 Subject: [PATCH 1/3] [clang-format] Properly indent lines inside Verilog case structure When a statement following a case label had to be broken into multiple lines, the continuation parts were not indented correctly. Old: ```Verilog case (data) 16'd0: result = // break here 10'b0111111111; endcase ``` New: ```Verilog case (data) 16'd0: result = // break here 10'b0111111111; endcase ``` Verilog case labels and the following statements are on the same unwrapped line due to the difficulty of identifying them. So there was a rule in `getNewLineColumn` to add a level of indentation to the part following the case label. However, in case the line had to be broken again, the code at the end of the function would see that the line was already broken with the continuation part indented, so it would not indent it more. Now `State.FirstIndent` is changed as well for the part following the case label, so the logic for determining when to add a continuation indentation works. --- clang/lib/Format/ContinuationIndenter.cpp | 22 ++++++--- clang/unittests/Format/FormatTestVerilog.cpp | 47 ++++++++++++++++++++ 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index ac62dab1b07cdca..ec21c181772fa81 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -1204,12 +1204,13 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) { CurrentState.Indent + Style.ContinuationIndentWidth); } - // After a goto label. Usually labels are on separate lines. However - // for Verilog the labels may be only recognized by the annotator and - // thus are on the same line as the current token. - if ((Style.isVerilog() && Keywords.isVerilogEndOfLabel(Previous)) || - (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths && - State.Line->First->is(tok::kw_enum))) { + // Indentation of the statement following a Verilog case label is taken care + // of in moveStateToNextToken. + if (Style.isVerilog() && Keywords.isVerilogEndOfLabel(Previous)) + return State.FirstIndent; + + if (Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths && + State.Line->First->is(tok::kw_enum)) { return (Style.IndentWidth * State.Line->First->IndentLevel) + Style.IndentWidth; } @@ -1599,6 +1600,15 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State, State.Column += Current.ColumnWidth; State.NextToken = State.NextToken->Next; + // Verilog case labels are are on the same unwrapped lines as the statements + // that follow. TokenAnnotator identifies them and sets MustBreakBefore. + // Indentation is taken care of here. A case label can only have 1 statement + // in Verilog, so we don't have to worry about lines that follow. + if (Style.isVerilog() && State.NextToken && + State.NextToken->MustBreakBefore && + Keywords.isVerilogEndOfLabel(Current)) { + State.FirstIndent += Style.IndentWidth; + } unsigned Penalty = handleEndOfLine(Current, State, DryRun, AllowBreak, Newline); diff --git a/clang/unittests/Format/FormatTestVerilog.cpp b/clang/unittests/Format/FormatTestVerilog.cpp index 9b090aa74f714d1..7161312126ea6ed 100644 --- a/clang/unittests/Format/FormatTestVerilog.cpp +++ b/clang/unittests/Format/FormatTestVerilog.cpp @@ -312,6 +312,53 @@ TEST_F(FormatTestVerilog, Case) { verifyFormat("default:\n" " x = '{x: x, default: 9};\n", Style); + // When the line following the case label needs to be indented, the + // continuation should be indented correctly. + verifyFormat("case (data)\n" + " 16'd0:\n" + " result = //\n" + " 10'b0111111111;\n" + "endcase"); + verifyFormat("case (data)\n" + " 16'd0, //\n" + " 16'd1:\n" + " result = //\n" + " 10'b0111111111;\n" + "endcase"); + verifyFormat("case (data)\n" + " 16'd0:\n" + " result = (10'b0111111111 + //\n" + " 10'b0111111111 + //\n" + " 10'b0111111111);\n" + "endcase"); + verifyFormat("case (data)\n" + " 16'd0:\n" + " result = //\n" + " (10'b0111111111 + //\n" + " 10'b0111111111 + //\n" + " 10'b0111111111);\n" + "endcase"); + verifyFormat("case (data)\n" + " 16'd0:\n" + " result = //\n" + " longfunction( //\n" + " arg);\n" + "endcase"); + Style = getDefaultStyle(); + Style.ContinuationIndentWidth = 1; + verifyFormat("case (data)\n" + " 16'd0:\n" + " result = //\n" + " 10'b0111111111;\n" + "endcase", + Style); + verifyFormat("case (data)\n" + " 16'd0:\n" + " result = //\n" + " longfunction( //\n" + " arg);\n" + "endcase", + Style); } TEST_F(FormatTestVerilog, Coverage) { >From d325e6b71f8d91eab0fac00ad162c7b782425999 Mon Sep 17 00:00:00 2001 From: sstwcw <su3e8a96kzl...@posteo.net> Date: Sun, 10 Sep 2023 02:17:13 +0000 Subject: [PATCH 2/3] Correct comment --- clang/lib/Format/ContinuationIndenter.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index ec21c181772fa81..dea39174d01c561 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -1600,9 +1600,9 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State, State.Column += Current.ColumnWidth; State.NextToken = State.NextToken->Next; - // Verilog case labels are are on the same unwrapped lines as the statements - // that follow. TokenAnnotator identifies them and sets MustBreakBefore. - // Indentation is taken care of here. A case label can only have 1 statement + // Verilog case labels are on the same unwrapped lines as the statements that + // follow. TokenAnnotator identifies them and sets MustBreakBefore. + // Indentation is taken care of here. A case label can only have 1 statement // in Verilog, so we don't have to worry about lines that follow. if (Style.isVerilog() && State.NextToken && State.NextToken->MustBreakBefore && >From 3b19b42bc0318e8ae3dd42ee346b68f4243a06a4 Mon Sep 17 00:00:00 2001 From: sstwcw <su3e8a96kzl...@posteo.net> Date: Sat, 16 Sep 2023 14:24:22 +0000 Subject: [PATCH 3/3] Correct comment --- clang/unittests/Format/FormatTestVerilog.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/unittests/Format/FormatTestVerilog.cpp b/clang/unittests/Format/FormatTestVerilog.cpp index 7161312126ea6ed..945e06143ccc3f1 100644 --- a/clang/unittests/Format/FormatTestVerilog.cpp +++ b/clang/unittests/Format/FormatTestVerilog.cpp @@ -312,8 +312,8 @@ TEST_F(FormatTestVerilog, Case) { verifyFormat("default:\n" " x = '{x: x, default: 9};\n", Style); - // When the line following the case label needs to be indented, the - // continuation should be indented correctly. + // When the line following the case label needs to be broken, the continuation + // should be indented correctly. verifyFormat("case (data)\n" " 16'd0:\n" " result = //\n" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits