[clang-format] Handle Verilog assertions and loops

[sstwcw]

Summary

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.

[HazardyKnusperkeks]

clang/unittests/Format/TokenAnnotatorTest.cpp

844

Unrelated, I'd prefer it as a single commit.

[sstwcw]

clang/unittests/Format/TokenAnnotatorTest.cpp

844

How should I name the separate commit?

[HazardyKnusperkeks]

clang/unittests/Format/TokenAnnotatorTest.cpp

844

I'd go with
[clang-format][NFC] Output tokens on test assert
or similar.

clang/lib/Format/TokenAnnotator.cpp

if ((Left.is(tok::l_paren) && Right.is(tok::r_paren)) ||
     return Style.SpaceInEmptyParentheses;
   }
   if (Style.SpacesInConditionalStatement) {
     const FormatToken *LeftParen = nullptr;
     if (Left.is(tok::l_paren))
       LeftParen = &Left;
     else if (Right.is(tok::r_paren) && Right.MatchingParen)
       LeftParen = Right.MatchingParen;
-    if (LeftParen && LeftParen->Previous &&
-        isKeywordWithCondition(*LeftParen->Previous)) {
+    if (LeftParen) {
+      if (LeftParen->is(TT_ConditionLParen))
+        return true;
+      if (LeftParen->Previous && isKeywordWithCondition(*LeftParen->Previous))
         return true;
     }
   }
 
   // trailing return type 'auto': []() -> auto {}, auto foo() -> auto {}
   if (Left.is(tok::kw_auto) && Right.isOneOf(TT_LambdaLBrace, TT_FunctionLBrace,
                                              // function return type 'auto'
                                              TT_FunctionTypeLParen)) {
     return true;

clang/lib/Format/UnwrappedLineParser.h

bool parseBracedList(bool ContinueOnSemicolons = false, bool IsEnum = false,
                        tok::TokenKind ClosingBraceKind = tok::r_brace);
   void parseParens(TokenType AmpAmpTokenType = TT_Unknown);
   void parseSquare(bool LambdaIntroducer = false);
   void keepAncestorBraces();
   void parseUnbracedBody(bool CheckEOF = false);
   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);
   void parseDoWhile();
   void parseLabel(bool LeftAlignLabel = false);
   void parseCaseLabel();
   void parseSwitch();
   void parseNamespace();

clang/lib/Format/UnwrappedLineParser.cpp

if (Style.Language == FormatStyle::LK_TableGen &&
     return;
   }
 
   if (Style.isVerilog()) {
     if (Keywords.isVerilogStructuredProcedure(*FormatTok)) {
       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) {
       if (FormatTok->isOneOf(Keywords.kw_priority, Keywords.kw_unique,
                              Keywords.kw_unique0)) {
         nextToken();
       } else if (FormatTok->is(tok::l_paren) &&
                  Tokens->peekNextToken()->is(tok::star)) {

 bool UnwrappedLineParser::isBlockBegin(const FormatToken &Tok) const {
   // FIXME: rename the function or make
   // Tok.isOneOf(tok::l_brace, TT_MacroBlockBegin) work.
   return Style.isVerilog() ? Keywords.isVerilogBegin(Tok)
                            : Tok.is(tok::l_brace);
 }
 
 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();
 
   bool KeepIfBraces = true;
   if (FormatTok->is(tok::kw_consteval)) {
     nextToken();
   } else {
     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();
 
   FormatToken *IfLeftBrace = nullptr;
   IfStmtKind IfBlockKind = IfStmtKind::NotIf;
 
   if (isBlockBegin(*FormatTok)) {
     FormatTok->setFinalizedType(TT_ControlStatementLBrace);
     IfLeftBrace = FormatTok;
     CompoundStatementIndenter Indenter(this, Style, Line->Level);
     parseBlock(/*MustBeDeclaration=*/false, /*AddLevels=*/1u,
                /*MunchSemi=*/true, KeepIfBraces, &IfBlockKind);
     if (Style.BraceWrapping.BeforeElse)
       addUnwrappedLine();
     else
       NeedsUnwrappedLine = true;
+  } else if (IsVerilogAssert && FormatTok->is(tok::kw_else)) {
+    addUnwrappedLine();
   } else {
     parseUnbracedBody();
   }
 
   if (Style.RemoveBracesLLVM) {
     assert(!NestedTooDeep.empty());
     KeepIfBraces = KeepIfBraces ||
                    (IfLeftBrace && !IfLeftBrace->MatchingParen) ||
if (isBlockBegin(*FormatTok)) {
                          ElseBlockKind == IfStmtKind::IfOnly ||
                          ElseBlockKind == IfStmtKind::IfElseIf;
       } else if (FollowedByIf && IfLBrace && !IfLBrace->Optional) {
         KeepElseBraces = true;
         assert(ElseLeftBrace->MatchingParen);
         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);
       if (IsPrecededByComment) {
         addUnwrappedLine();
         ++Line->Level;
       }
       bool TooDeep = true;
const bool KeepBraces = !Style.RemoveBracesLLVM ||
                           !FormatTok->isOneOf(tok::kw_for, tok::kw_while);
 
   nextToken();
   // JS' for await ( ...
   if (Style.isJavaScript() && FormatTok->is(Keywords.kw_await))
     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();
 
   handleAttributes();
   parseLoopBody(KeepBraces, /*WrapRightBrace=*/true);
 }
 

clang/unittests/Format/FormatTestVerilog.cpp

TEST_F(FormatTestVerilog, If) {
 
   // With attributes.
   verifyFormat("(* x *) if (x)\n"
                "  x = x;");
   verifyFormat("(* x = \"x\" *) if (x)\n"
                "  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) {
   // Without ports.
   verifyFormat("ffnand ff1;");
   // With named ports.
   verifyFormat("ffnand ff1(.qbar(out1),\n"
                "           .clear(in1),\n"
verifyFormat("ffnand ff1(.q(), .qbar(out1), .clear(in1), .preset(in2)),\n"
                "       ff1(.q(), .qbar(out1), .clear(in1), .preset(in2));",
                Style);
   verifyFormat("ffnand //\n"
                "    ff1(.q(), .qbar(out1), .clear(in1), .preset(in2)),\n"
                "    ff1(.q(), .qbar(out1), .clear(in1), .preset(in2));",
                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;");
   verifyFormat("x = -x;");
   verifyFormat("x = !x;");
   verifyFormat("x = ~x;");
   verifyFormat("x = &x;");
   verifyFormat("x = ~&x;");

clang/unittests/Format/TokenAnnotatorTest.cpp

TEST_F(TokenAnnotatorTest, UnderstandsRequiresClausesAndConcepts) {
   EXPECT_TOKEN(Tokens[7], tok::kw_requires, TT_RequiresClause);
 
   Tokens = annotate("struct [[nodiscard]] zero_t {\n"
                     "  template<class T>\n"
                     "    requires requires { number_zero_v<T>; }\n"
                     "  [[nodiscard]] constexpr operator T() const { "
                     "return number_zero_v<T>; }\n"
                     "};");
   ASSERT_EQ(Tokens.size(), 44u);
   EXPECT_TOKEN(Tokens[13], tok::kw_requires, TT_RequiresClause);
   EXPECT_TOKEN(Tokens[14], tok::kw_requires, TT_RequiresExpression);
   EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[21], tok::r_brace, TT_Unknown);
   EXPECT_EQ(Tokens[21]->MatchingParen, Tokens[15]);
   EXPECT_TRUE(Tokens[21]->ClosesRequiresClause);
 
   Tokens =
TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_VerilogInstancePortLParen);
   EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_VerilogInstancePortLParen);
   Tokens = Annotate("module_x #(parameter) instance_1(port_1), "
                     "instance_2(port_2);");
   ASSERT_EQ(Tokens.size(), 16u);
   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) {
   auto Tokens = annotate("Class::Class() : BaseClass(), Member() {}");
 
   // The TT_Unknown is clearly not binding for the future, please adapt if those
   // tokens get annotated.
   ASSERT_EQ(Tokens.size(), 16u) << Tokens;
TEST_F(TokenAnnotatorTest, UnderstandConstructors) {
   EXPECT_TOKEN(Tokens[8], tok::r_brace, TT_Unknown);
   EXPECT_TOKEN(Tokens[9], tok::comma, TT_CtorInitializerComma);
   EXPECT_TOKEN(Tokens[10], tok::identifier, TT_Unknown);
   EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_Unknown);
   EXPECT_TOKEN(Tokens[12], tok::r_brace, TT_Unknown);
   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