[clang] 0571ba8 - [clang-format] Handle Verilog assertions and loops
via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 16 15:00:34 PDT 2023
Author: sstwcw
Date: 2023-04-16T21:55:50Z
New Revision: 0571ba8d1b4dbe72e0856f9b84146cc6e152a4d0
URL: https://github.com/llvm/llvm-project/commit/0571ba8d1b4dbe72e0856f9b84146cc6e152a4d0
DIFF: https://github.com/llvm/llvm-project/commit/0571ba8d1b4dbe72e0856f9b84146cc6e152a4d0.diff
LOG: [clang-format] Handle Verilog assertions and loops
Assert statements in Verilog can optionally have an else part. We
handle them like for `if` statements, except that an `if` statement in
the else part of an `assert` statement doesn't get merged with the
`else` keyword. Like this:
assert (x)
$info();
else
if (y)
$info();
else if (z)
$info();
else
$info();
`foreach` and `repeat` are now handled like for or while loops.
We used the type `TT_ConditionLParen` to mark the condition part so
they are handled in the same way as the condition part of an `if`
statement. When the code being formatted is not in Verilog, it is
only set for `if` statements, not loops. It's because loop conditions
are currently handled slightly differently, and existing behavior is
not supposed to change. We formatted all files ending in `.cpp` and
`.h` in the repository with and without this change. It showed that
setting the type for `if` statements doesn't change existing behavior.
And we noticed that we forgot to make the program print the list of
tokens when the number is not correct in `TokenAnnotatorTest`. It's
fixed now.
Reviewed By: HazardyKnusperkeks
Differential Revision: https://reviews.llvm.org/D147895
Added:
Modified:
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/UnwrappedLineParser.h
clang/unittests/Format/FormatTestVerilog.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 9060bf76a7f52..28e21f93c14c4 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3700,9 +3700,11 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
LeftParen = &Left;
else if (Right.is(tok::r_paren) && Right.MatchingParen)
LeftParen = Right.MatchingParen;
- if (LeftParen && LeftParen->Previous &&
- isKeywordWithCondition(*LeftParen->Previous)) {
- return true;
+ if (LeftParen) {
+ if (LeftParen->is(TT_ConditionLParen))
+ return true;
+ if (LeftParen->Previous && isKeywordWithCondition(*LeftParen->Previous))
+ return true;
}
}
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 9158333911ae2..5c6816d6df30f 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1393,6 +1393,15 @@ void UnwrappedLineParser::parseStructuralElement(
parseForOrWhileLoop(/*HasParens=*/false);
return;
}
+ if (FormatTok->isOneOf(Keywords.kw_foreach, Keywords.kw_repeat)) {
+ parseForOrWhileLoop();
+ return;
+ }
+ if (FormatTok->isOneOf(tok::kw_restrict, Keywords.kw_assert,
+ Keywords.kw_assume, Keywords.kw_cover)) {
+ parseIfThenElse(IfKind, /*KeepBraces=*/false, /*IsVerilogAssert=*/true);
+ return;
+ }
// Skip things that can exist before keywords like 'if' and 'case'.
while (true) {
@@ -2624,9 +2633,28 @@ bool UnwrappedLineParser::isBlockBegin(const FormatToken &Tok) const {
}
FormatToken *UnwrappedLineParser::parseIfThenElse(IfStmtKind *IfKind,
- bool KeepBraces) {
- assert(FormatTok->is(tok::kw_if) && "'if' expected");
+ bool KeepBraces,
+ bool IsVerilogAssert) {
+ assert((FormatTok->is(tok::kw_if) ||
+ (Style.isVerilog() &&
+ FormatTok->isOneOf(tok::kw_restrict, Keywords.kw_assert,
+ Keywords.kw_assume, Keywords.kw_cover))) &&
+ "'if' expected");
nextToken();
+
+ if (IsVerilogAssert) {
+ // Handle `assert #0` and `assert final`.
+ if (FormatTok->is(Keywords.kw_verilogHash)) {
+ nextToken();
+ if (FormatTok->is(tok::numeric_constant))
+ nextToken();
+ } else if (FormatTok->isOneOf(Keywords.kw_final, Keywords.kw_property,
+ Keywords.kw_sequence)) {
+ nextToken();
+ }
+ }
+
+ // Handle `if !consteval`.
if (FormatTok->is(tok::exclaim))
nextToken();
@@ -2637,10 +2665,18 @@ FormatToken *UnwrappedLineParser::parseIfThenElse(IfStmtKind *IfKind,
KeepIfBraces = !Style.RemoveBracesLLVM || KeepBraces;
if (FormatTok->isOneOf(tok::kw_constexpr, tok::identifier))
nextToken();
- if (FormatTok->is(tok::l_paren))
+ if (FormatTok->is(tok::l_paren)) {
+ FormatTok->setFinalizedType(TT_ConditionLParen);
parseParens();
+ }
}
handleAttributes();
+ // The then action is optional in Verilog assert statements.
+ if (IsVerilogAssert && FormatTok->is(tok::semi)) {
+ nextToken();
+ addUnwrappedLine();
+ return nullptr;
+ }
bool NeedsUnwrappedLine = false;
keepAncestorBraces();
@@ -2658,6 +2694,8 @@ FormatToken *UnwrappedLineParser::parseIfThenElse(IfStmtKind *IfKind,
addUnwrappedLine();
else
NeedsUnwrappedLine = true;
+ } else if (IsVerilogAssert && FormatTok->is(tok::kw_else)) {
+ addUnwrappedLine();
} else {
parseUnbracedBody();
}
@@ -2700,7 +2738,7 @@ FormatToken *UnwrappedLineParser::parseIfThenElse(IfStmtKind *IfKind,
markOptionalBraces(ElseLeftBrace);
}
addUnwrappedLine();
- } else if (FormatTok->is(tok::kw_if)) {
+ } else if (!IsVerilogAssert && FormatTok->is(tok::kw_if)) {
const FormatToken *Previous = Tokens->getPreviousToken();
assert(Previous);
const bool IsPrecededByComment = Previous->is(tok::comment);
@@ -2993,8 +3031,14 @@ void UnwrappedLineParser::parseForOrWhileLoop(bool HasParens) {
nextToken();
if (Style.isCpp() && FormatTok->is(tok::kw_co_await))
nextToken();
- if (HasParens && FormatTok->is(tok::l_paren))
+ if (HasParens && FormatTok->is(tok::l_paren)) {
+ // The type is only set for Verilog basically because we were afraid to
+ // change the existing behavior for loops. See the discussion on D121756 for
+ // details.
+ if (Style.isVerilog())
+ FormatTok->setFinalizedType(TT_ConditionLParen);
parseParens();
+ }
// Event control.
if (Style.isVerilog())
parseVerilogSensitivityList();
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index 4a48fa856247a..9650ce1a9c158 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -163,7 +163,8 @@ class UnwrappedLineParser {
void handleAttributes();
bool handleCppAttributes();
bool isBlockBegin(const FormatToken &Tok) const;
- FormatToken *parseIfThenElse(IfStmtKind *IfKind, bool KeepBraces = false);
+ FormatToken *parseIfThenElse(IfStmtKind *IfKind, bool KeepBraces = false,
+ bool IsVerilogAssert = false);
void parseTryCatch();
void parseLoopBody(bool KeepBraces, bool WrapRightBrace);
void parseForOrWhileLoop(bool HasParens = true);
diff --git a/clang/unittests/Format/FormatTestVerilog.cpp b/clang/unittests/Format/FormatTestVerilog.cpp
index 5e1d8649480b4..ff6a1cdc57fe7 100644
--- a/clang/unittests/Format/FormatTestVerilog.cpp
+++ b/clang/unittests/Format/FormatTestVerilog.cpp
@@ -671,6 +671,107 @@ TEST_F(FormatTestVerilog, If) {
" x = x;");
verifyFormat("(* x, x = \"x\" *) if (x)\n"
" x = x;");
+
+ // Assert are treated similar to if. But the else parts should not be
+ // chained.
+ verifyFormat("assert (x);");
+ verifyFormat("assert (x)\n"
+ " $info();");
+ verifyFormat("assert (x)\n"
+ " $info();\n"
+ "else\n"
+ " $error();");
+ verifyFormat("assert (x)\n"
+ "else\n"
+ " $error();");
+ verifyFormat("assert (x)\n"
+ "else begin\n"
+ "end");
+ verifyFormat("assert (x)\n"
+ "else\n"
+ " if (x)\n"
+ " $error();");
+ verifyFormat("assert (x)\n"
+ " $info();\n"
+ "else\n"
+ " if (x)\n"
+ " $error();");
+ verifyFormat("assert (x)\n"
+ " $info();\n"
+ "else\n"
+ " if (x)\n"
+ " $error();\n"
+ " else\n"
+ " $error();");
+ verifyFormat("assert (x)\n"
+ " $info();\n"
+ "else\n"
+ " if (x)\n"
+ " $error();\n"
+ " else if (x)\n"
+ " $error();\n"
+ " else\n"
+ " $error();");
+ // The body is optional for asserts. The next line should not be indented if
+ // the statement already ended with a semicolon.
+ verifyFormat("assert (x);\n"
+ "x = x;");
+ verifyFormat("if (x)\n"
+ " assert (x);\n"
+ "else if (x) begin\n"
+ "end else begin\n"
+ "end");
+ verifyFormat("if (x)\n"
+ " assert (x);\n"
+ "else begin\n"
+ "end");
+ verifyFormat("if (x)\n"
+ " assert (x)\n"
+ " else begin\n"
+ " end");
+ // Other keywords.
+ verifyFormat("assume (x)\n"
+ " $info();");
+ verifyFormat("cover (x)\n"
+ " $info();");
+ verifyFormat("restrict (x)\n"
+ " $info();");
+ verifyFormat("assert #0 (x)\n"
+ " $info();");
+ verifyFormat("assert final (x)\n"
+ " $info();");
+ verifyFormat("cover #0 (x)\n"
+ " $info();");
+ verifyFormat("cover final (x)\n"
+ " $info();");
+
+ // The space around parentheses options should work.
+ auto Style = getDefaultStyle();
+ verifyFormat("if (x)\n"
+ " x = x;\n"
+ "else if (x)\n"
+ " x = x;",
+ Style);
+ verifyFormat("assert (x);", Style);
+ verifyFormat("assert #0 (x);", Style);
+ verifyFormat("assert (x)\n"
+ "else\n"
+ " if (x)\n"
+ " x = x;",
+ Style);
+ Style.SpacesInConditionalStatement = true;
+ verifyFormat("if ( x )\n"
+ " x = x;\n"
+ "else if ( x )\n"
+ " x = x;",
+ Style);
+ verifyFormat("assert ( x );", Style);
+ verifyFormat("assert #0 ( x );", Style);
+ verifyFormat("assert ( x )\n"
+ "else\n"
+ " if ( x )\n"
+ " x = x;",
+ Style);
}
TEST_F(FormatTestVerilog, Instantiation) {
@@ -744,6 +845,25 @@ TEST_F(FormatTestVerilog, Instantiation) {
Style);
}
+TEST_F(FormatTestVerilog, Loop) {
+ verifyFormat("foreach (x[x])\n"
+ " x = x;");
+ verifyFormat("repeat (x)\n"
+ " x = x;");
+ verifyFormat("foreach (x[x]) begin\n"
+ "end");
+ verifyFormat("repeat (x) begin\n"
+ "end");
+ auto Style = getDefaultStyle();
+ Style.SpacesInConditionalStatement = true;
+ verifyFormat("foreach ( x[x] )\n"
+ " x = x;",
+ Style);
+ verifyFormat("repeat ( x )\n"
+ " x = x;",
+ Style);
+}
+
TEST_F(FormatTestVerilog, Operators) {
// Test that unary operators are not followed by space.
verifyFormat("x = +x;");
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 2d1d7749a8e8d..ce37abed595d5 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1657,6 +1657,26 @@ TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_VerilogInstancePortLParen);
EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_VerilogInstancePortLParen);
EXPECT_TOKEN(Tokens[11], tok::l_paren, TT_VerilogInstancePortLParen);
+
+ // Condition parentheses.
+ Tokens = Annotate("assert (x);");
+ ASSERT_EQ(Tokens.size(), 6u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ConditionLParen);
+ Tokens = Annotate("assert #0 (x);");
+ ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+ EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_ConditionLParen);
+ Tokens = Annotate("assert final (x);");
+ ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+ EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_ConditionLParen);
+ Tokens = Annotate("foreach (x[x]) continue;");
+ ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ConditionLParen);
+ Tokens = Annotate("repeat (x[x]) continue;");
+ ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ConditionLParen);
+ Tokens = Annotate("case (x) endcase;");
+ ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ConditionLParen);
}
TEST_F(TokenAnnotatorTest, UnderstandConstructors) {
@@ -1688,6 +1708,22 @@ TEST_F(TokenAnnotatorTest, UnderstandConstructors) {
EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
}
+TEST_F(TokenAnnotatorTest, UnderstandsConditionParens) {
+ auto Tokens = annotate("if (x) {}");
+ ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ConditionLParen);
+ Tokens = annotate("if constexpr (x) {}");
+ ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+ EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_ConditionLParen);
+ Tokens = annotate("if CONSTEXPR (x) {}");
+ ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+ EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_ConditionLParen);
+ Tokens = annotate("if (x) {} else if (x) {}");
+ ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+ EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_ConditionLParen);
+ EXPECT_TOKEN(Tokens[8], tok::l_paren, TT_ConditionLParen);
+}
+
} // namespace
} // namespace format
} // namespace clang
More information about the cfe-commits
mailing list