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] Support of TableGen basic format restrictions. #81611

Merged
merged 6 commits into from
Feb 16, 2024

Conversation

hnakamura5
Copy link
Contributor

@hnakamura5 hnakamura5 commented Feb 13, 2024

  • Allow/force to break the line or not.
  • Allow/force to insert space or not.

This is separated part from #76059.
Now we come to format in basic style !

This PR seems large, but the most part is the formatting unittest.

- Allow/force to break the line or not.
- Allow to insert space or not.
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 13, 2024

@llvm/pr-subscribers-clang-format

Author: Hirofumi Nakamura (hnakamura5)

Changes
  • Allow/force to break the line or not.
  • Allow to insert space or not.

This is separated part from #76059.
Now we come to format in basic style !


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

3 Files Affected:

  • (modified) clang/lib/Format/ContinuationIndenter.cpp (+12-2)
  • (modified) clang/lib/Format/TokenAnnotator.cpp (+56)
  • (modified) clang/unittests/Format/FormatTestTableGen.cpp (+263)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 0b2ef97af44d83..1879af94f6da49 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -821,6 +821,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
   if (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign &&
       !CurrentState.IsCSharpGenericTypeConstraint && Previous.opensScope() &&
       Previous.isNot(TT_ObjCMethodExpr) && Previous.isNot(TT_RequiresClause) &&
+      Previous.isNot(TT_TableGenDAGArgOpener) &&
       !(Current.MacroParent && Previous.MacroParent) &&
       (Current.isNot(TT_LineComment) ||
        Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen))) {
@@ -1250,7 +1251,7 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
     return CurrentState.Indent;
   }
   if ((Current.isOneOf(tok::r_brace, tok::r_square) ||
-       (Current.is(tok::greater) && Style.isProto())) &&
+       (Current.is(tok::greater) && (Style.isProto() || Style.isTableGen()))) &&
       State.Stack.size() > 1) {
     if (Current.closesBlockOrBlockTypeList(Style))
       return State.Stack[State.Stack.size() - 2].NestedBlockIndent;
@@ -1278,6 +1279,12 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
        Current.Next->isOneOf(tok::semi, tok::kw_const, tok::l_brace))) {
     return State.Stack[State.Stack.size() - 2].LastSpace;
   }
+  // When DAGArg closer exists top of line, it should be aligned in the similar
+  // way as function call above.
+  if (Style.isTableGen() && Current.is(TT_TableGenDAGArgCloser) &&
+      State.Stack.size() > 1) {
+    return State.Stack[State.Stack.size() - 2].LastSpace;
+  }
   if (Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent &&
       (Current.is(tok::r_paren) ||
        (Current.is(tok::r_brace) && Current.MatchingParen &&
@@ -1696,7 +1703,9 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
         (!Previous || Previous->isNot(tok::kw_return) ||
          (Style.Language != FormatStyle::LK_Java && PrecedenceLevel > 0)) &&
         (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign ||
-         PrecedenceLevel != prec::Comma || Current.NestingLevel == 0)) {
+         PrecedenceLevel != prec::Comma || Current.NestingLevel == 0) &&
+        (!Style.isTableGen() ||
+         (Previous && Previous->is(TT_TableGenDAGArgListComma)))) {
       NewParenState.Indent = std::max(
           std::max(State.Column, NewParenState.Indent), CurrentState.LastSpace);
     }
@@ -1942,6 +1951,7 @@ void ContinuationIndenter::moveStatePastScopeCloser(LineState &State) {
       (Current.isOneOf(tok::r_paren, tok::r_square, TT_TemplateString) ||
        (Current.is(tok::r_brace) && State.NextToken != State.Line->First) ||
        State.NextToken->is(TT_TemplateCloser) ||
+       State.NextToken->is(TT_TableGenListCloser) ||
        (Current.is(tok::greater) && Current.is(TT_DictLiteral)))) {
     State.Stack.pop_back();
   }
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index d353388a862b56..636d098881c97e 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5072,7 +5072,38 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
          Left.endsSequence(tok::greatergreater, tok::l_brace))) {
       return false;
     }
+  } else if (Style.isTableGen()) {
+    // Avoid to connect [ and {. [{ is start token of multiline string.
+    if (Left.is(tok::l_square) && Right.is(tok::l_brace))
+      return true;
+    if (Left.is(tok::r_brace) && Right.is(tok::r_square))
+      return true;
+    // Do not insert around colon in DAGArg and cond operator.
+    if (Right.is(TT_TableGenDAGArgListColon) ||
+        Left.is(TT_TableGenDAGArgListColon)) {
+      return false;
+    }
+    if (Right.is(TT_TableGenCondOperatorColon))
+      return false;
+    // Do not insert bang operators and consequent openers.
+    if (Right.isOneOf(tok::l_paren, tok::greater) &&
+        Left.isOneOf(TT_TableGenBangOperator, TT_TableGenCondOperator)) {
+      return false;
+    }
+    // Trailing paste requires space before '{' or ':', the case in name values.
+    // Not before ';', the case in normal values.
+    if (Left.is(TT_TableGenTrailingPasteOperator) &&
+        Right.isOneOf(tok::l_brace, tok::colon)) {
+      return true;
+    }
+    // Otherwise paste operator does not prefer space around.
+    if (Left.is(tok::hash) || Right.is(tok::hash))
+      return false;
+    // Sure not to connect after defining keywords.
+    if (Keywords.isTableGenDefinition(Left))
+      return true;
   }
+
   if (Left.is(TT_ImplicitStringLiteral))
     return Right.hasWhitespaceBefore();
   if (Line.Type == LT_ObjCMethodDecl) {
@@ -5424,6 +5455,13 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
       return Style.BreakArrays;
     }
   }
+  if (Style.isTableGen()) {
+    // Break the comma in side cond operators.
+    // !cond(case1:1,
+    //       case2:0);
+    if (Left.is(TT_TableGenCondOperatorComma))
+      return true;
+  }
 
   if (Line.startsWith(tok::kw_asm) && Right.is(TT_InlineASMColon) &&
       Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always) {
@@ -5822,6 +5860,24 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
       return false;
     if (Left.is(TT_TemplateString) && Left.opensScope())
       return true;
+  } else if (Style.isTableGen()) {
+    // Avoid to break after "def", "class", "let" and so on.
+    if (Keywords.isTableGenDefinition(Left))
+      return false;
+    // Avoid to break after '(' in the cases that is in bang operators.
+    if (Right.is(tok::l_paren)) {
+      return !Left.isOneOf(TT_TableGenBangOperator, TT_TableGenCondOperator,
+                           TT_TemplateCloser);
+    }
+    // Avoid to split the value and its suffix part.
+    if (Left.is(TT_TableGenValueSuffix))
+      return false;
+    // Avoid to break between the value and its suffix part.
+    if (Left.is(TT_TableGenValueSuffix))
+      return false;
+    // Avoid to break around paste operator.
+    if (Left.is(tok::hash) || Right.is(tok::hash))
+      return false;
   }
 
   if (Left.is(tok::at))
diff --git a/clang/unittests/Format/FormatTestTableGen.cpp b/clang/unittests/Format/FormatTestTableGen.cpp
index 123f47d6d7f884..77ddb4ed228864 100644
--- a/clang/unittests/Format/FormatTestTableGen.cpp
+++ b/clang/unittests/Format/FormatTestTableGen.cpp
@@ -55,5 +55,268 @@ TEST_F(FormatTestTableGen, NoSpacesInSquareBracketLists) {
   verifyFormat("def flag : Flag<[\"-\", \"--\"], \"foo\">;");
 }
 
+TEST_F(FormatTestTableGen, LiteralsAndIdentifiers) {
+  verifyFormat("def LiteralAndIdentifiers {\n"
+               "  let someInteger = -42;\n"
+               "  let 0startID = $TokVarName;\n"
+               "  let 0xstartInteger = 0x42;\n"
+               "  let someIdentifier = $TokVarName;\n"
+               "}\n");
+}
+
+TEST_F(FormatTestTableGen, BangOperators) {
+  verifyFormat("def BangOperators {\n"
+               "  let IfOpe = !if(\n"
+               "      !not(!and(!gt(!add(1, 2), !sub(3, 4)), !isa<Ty>($x))),\n"
+               "      !foldl(0, !listconcat(!range(5, 6), !range(7, 8)),\n"
+               "             total, rec, !add(total, rec.Number)),\n"
+               "      !tail(!range(9, 10)));\n"
+               "  let ForeachOpe = !foreach(\n"
+               "      arg, arglist,\n"
+               "      !if(!isa<SomeType>(arg.Type),\n"
+               "          !add(!cast<SomeOtherType>(arg).Number, x), arg));\n"
+               "  let CondOpe1 = !cond(!eq(size, 1): 1,\n"
+               "                       !eq(size, 2): 1,\n"
+               "                       !eq(size, 4): 1,\n"
+               "                       !eq(size, 8): 1,\n"
+               "                       !eq(size, 16): 1,\n"
+               "                       true: 0);\n"
+               "  let CondOpe2 = !cond(!lt(x, 0): \"negativenegative\",\n"
+               "                       !eq(x, 0): \"zerozero\",\n"
+               "                       true: \"positivepositive\");\n"
+               "  let CondOpe2WithComment = !cond(!lt(x, 0):  // negative\n"
+               "                                  \"negativenegative\",\n"
+               "                                  !eq(x, 0):  // zero\n"
+               "                                  \"zerozero\",\n"
+               "                                  true:  // default\n"
+               "                                  \"positivepositive\");\n"
+               "}\n");
+}
+
+TEST_F(FormatTestTableGen, Include) {
+  verifyFormat("include \"test/IncludeFile.h\"\n");
+}
+
+TEST_F(FormatTestTableGen, Types) {
+  verifyFormat("def Types : list<int>, bits<3>, list<list<string>> {}\n");
+}
+
+TEST_F(FormatTestTableGen, SimpleValue1_SingleLiterals) {
+  verifyFormat("def SimpleValue {\n"
+               "  let Integer = 42;\n"
+               "  let String = \"some string\";\n"
+               "}\n");
+}
+
+TEST_F(FormatTestTableGen, SimpleValue1_MultilineString) {
+  // verifyFormat does not understand multiline TableGen code-literals
+  std::string DefWithCode =
+      "def SimpleValueCode {\n"
+      "  let Code =\n"
+      "      [{ A TokCode is  nothing more than a multi-line string literal "
+      "delimited by \\[{ and }\\]. It  can break across lines and the line "
+      "breaks are retained in the string. "
+      "(https://llvm.org/docs/TableGen/ProgRef.html#grammar-token-TokCode)}];\n"
+      "}\n";
+  std::string DefWithCodeMessingUp =
+      "def SimpleValueCode {\n"
+      "  let   Code=       "
+      "[{ A TokCode is  nothing more than a multi-line string literal "
+      "delimited by \\[{ and }\\]. It  can break across lines and the line "
+      "breaks are retained in the string. "
+      "(https://llvm.org/docs/TableGen/ProgRef.html#grammar-token-TokCode)}];\n"
+      "   }    \n";
+  EXPECT_EQ(DefWithCode, format(DefWithCodeMessingUp));
+}
+
+TEST_F(FormatTestTableGen, SimpleValue2) {
+  verifyFormat("def SimpleValue2 {\n"
+               "  let True = true;\n"
+               "  let False = false;\n"
+               "}\n");
+}
+
+TEST_F(FormatTestTableGen, SimpleValue3) {
+  verifyFormat("class SimpleValue3<int x> { int Question = ?; }\n");
+}
+
+TEST_F(FormatTestTableGen, SimpleValue4) {
+  verifyFormat("def SimpleValue4 { let ValueList = {1, 2, 3}; }\n");
+}
+
+TEST_F(FormatTestTableGen, SimpleValue5) {
+  verifyFormat("def SimpleValue5 {\n"
+               "  let SquareList = [1, 4, 9];\n"
+               "  let SquareListWithType = [\"a\", \"b\", \"c\"]<string>;\n"
+               "  let SquareListListWithType = [[1, 2], [3, 4, 5], [7]]<\n"
+               "      list<int>>;\n"
+               "  let SquareBitsListWithType = [ {1, 2},\n"
+               "                                 {3, 4} ]<list<bits<8>>>;\n"
+               "}\n");
+}
+
+TEST_F(FormatTestTableGen, SimpleValue6) {
+  verifyFormat("def SimpleValue6 {\n"
+               "  let DAGArgIns = (ins i32:$src1, i32:$src2);\n"
+               "  let DAGArgOuts = (outs i32:$dst1, i32:$dst2, i32:$dst3,\n"
+               "      i32:$dst4, i32:$dst5, i32:$dst6, i32:$dst7);\n"
+               "  let DAGArgOutsWithComment = (outs i32:$dst1,  // dst1\n"
+               "      i32:$dst2,                                // dst2\n"
+               "      i32:$dst3,                                // dst3\n"
+               "      i32:$dst4,                                // dst4\n"
+               "      i32:$dst5,                                // dst5\n"
+               "      i32:$dst6,                                // dst6\n"
+               "      i32:$dst7                                 // dst7\n"
+               "  );\n"
+               "  let DAGArgBang = (!cast<SomeType>(\"Some\") i32:$src1,\n"
+               "      i32:$src2);\n"
+               "}\n");
+}
+
+TEST_F(FormatTestTableGen, SimpleValue7) {
+  verifyFormat("def SimpleValue7 { let Identifier = SimpleValue; }\n");
+}
+
+TEST_F(FormatTestTableGen, SimpleValue8) {
+  verifyFormat("def SimpleValue8 { let Class = SimpleValue3<3>; }\n");
+}
+
+TEST_F(FormatTestTableGen, ValueSuffix) {
+  verifyFormat("def SuffixedValues {\n"
+               "  let Bit = value{17};\n"
+               "  let Bits = value{8...15};\n"
+               "  let List = value[1];\n"
+               "  let Slice1 = value[1, ];\n"
+               "  let Slice2 = value[4...7, 17, 2...3, 4];\n"
+               "  let Field = value.field;\n"
+               "}\n");
+}
+
+TEST_F(FormatTestTableGen, PasteOperator) {
+  verifyFormat(
+      "def Paste#\"Operator\" { string Paste = \"Paste\"#operator; }\n");
+
+  verifyFormat("def [\"Traring\", \"Paste\"]# {\n"
+               "  string X = Traring#;\n"
+               "  string Y = List<\"Operator\">#;\n"
+               "  string Z = [\"Traring\", \"Paste\", \"Traring\", \"Paste\",\n"
+               "              \"Traring\", \"Paste\"]#;\n"
+               "}\n");
+}
+
+TEST_F(FormatTestTableGen, ClassDefinition) {
+  verifyFormat("class Class<int x, int y = 1, string z = \"z\", int w = -1>\n"
+               "    : Parent1, Parent2<x, y> {\n"
+               "  int Item1 = 1;\n"
+               "  int Item2;\n"
+               "  code Item3 = [{ Item3 }];\n"
+               "  let Item4 = 4;\n"
+               "  let Item5{1, 2} = 5;\n"
+               "  defvar Item6 = 6;\n"
+               "  let Item7 = ?;\n"
+               "  assert !ge(x, 0), \"Assert7\";\n"
+               "}\n");
+
+  verifyFormat("class FPFormat<bits<3> val> { bits<3> Value = val; }\n");
+}
+
+TEST_F(FormatTestTableGen, Def) {
+  verifyFormat("def Def : Parent1<Def>, Parent2(defs Def) {\n"
+               "  code Item1 = [{ Item1 }];\n"
+               "  let Item2{1, 3...4} = {1, 2};\n"
+               "  defvar Item3 = (ops nodty:$node1, nodty:$node2);\n"
+               "  assert !le(Item2, 0), \"Assert4\";\n"
+               "}\n");
+
+  verifyFormat("class FPFormat<bits<3> val> { bits<3> Value = val; }\n");
+
+  verifyFormat("def NotFP : FPFormat<0>;\n");
+}
+
+TEST_F(FormatTestTableGen, Let) {
+  verifyFormat("let x = 1, y = value<type>,\n"
+               "    z = !and(!gt(!add(1, 2), !sub(3, 4)), !isa<Ty>($x)) in {\n"
+               "  class Class1 : Parent<x, y> { let Item1 = z; }\n"
+               "}\n");
+}
+
+TEST_F(FormatTestTableGen, MultiClass) {
+  verifyFormat("multiclass Multiclass<int x> {\n"
+               "  def : Def1<(item type:$src1),\n"
+               "             (!if(!ge(x, 0), !mul(!add(x, 1), !sub(x, 2)),\n"
+               "                  !sub(x, 2)))>;\n"
+               "  def Def2 : value<type>;\n"
+               "  def Def3 : type { let value = 1; }\n"
+               "  defm : SomeMultiClass<Def1, Def2>;\n"
+               "  defvar DefVar = 6;\n"
+               "  foreach i = [1, 2, 3] in {\n"
+               "    def : Foreach#i<(item type:$src1),\n"
+               "                    (!if(!gt(x, i),\n"
+               "                         !mul(!add(x, i), !sub(x, i)),\n"
+               "                         !sub(x, !add(i, 1))))>;\n"
+               "  }\n"
+               "  if !gt(x, 0) then {\n"
+               "    def : IfThen<x>;\n"
+               "  } else {\n"
+               "    def : IfElse<x>;\n"
+               "  }\n"
+               "  if (dagid x, 0) then {\n"
+               "    def : If2<1>;\n"
+               "  }\n"
+               "  let y = 1, z = 2 in {\n"
+               "    multiclass Multiclass2<int x> {\n"
+               "      foreach i = [1, 2, 3] in {\n"
+               "        def : Foreach#i<(item type:$src1),\n"
+               "                        (!if(!gt(z, i),\n"
+               "                             !mul(!add(y, i), !sub(x, i)),\n"
+               "                             !sub(z, !add(i, 1))))>;\n"
+               "      }\n"
+               "    }\n"
+               "  }\n"
+               "}\n");
+}
+
+TEST_F(FormatTestTableGen, Defm) {
+  verifyFormat("defm : Multiclass<0>;\n");
+
+  verifyFormat("defm Defm1 : Multiclass<1>;\n");
+}
+
+TEST_F(FormatTestTableGen, Defset) {
+  verifyFormat("defset list<Class> DefSet1 = {\n"
+               "  def Def1 : Class<1>;\n"
+               "  def Def2 : Class<2>;\n"
+               "}\n");
+}
+
+TEST_F(FormatTestTableGen, Defvar) {
+  verifyFormat("defvar DefVar1 = !cond(!ge(!size(PaseOperator.Paste), 1): 1,\n"
+               "                       true: 0);\n");
+}
+
+TEST_F(FormatTestTableGen, ForEach) {
+  verifyFormat(
+      "foreach i = [1, 2, 3] in {\n"
+      "  def : Foreach#i<(item type:$src1),\n"
+      "                  (!if(!lt(x, i),\n"
+      "                       !shl(!mul(x, i), !size(\"string\")),\n"
+      "                       !size(!strconcat(\"a\", \"b\", \"c\"))))>;\n"
+      "}\n");
+}
+
+TEST_F(FormatTestTableGen, Dump) { verifyFormat("dump \"Dump\";\n"); }
+
+TEST_F(FormatTestTableGen, If) {
+  verifyFormat("if !gt(x, 0) then {\n"
+               "  def : IfThen<x>;\n"
+               "} else {\n"
+               "  def : IfElse<x>;\n"
+               "}\n");
+}
+
+TEST_F(FormatTestTableGen, Assert) {
+  verifyFormat("assert !le(DefVar1, 0), \"Assert1\";\n");
+}
+
 } // namespace format
 } // end namespace clang

" list<int>>;\n"
" let SquareBitsListWithType = [ {1, 2},\n"
" {3, 4} ]<list<bits<8>>>;\n"
"}\n");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SquareListListWithType and SquareBitsListWithType seems a little bit strange for they are similar but the format is difference.
One reason is '[' and '{' cannot be connected because '[{' is the beginning of multiline string.

return false;
// Avoid to break around paste operator.
if (Left.is(tok::hash) || Right.is(tok::hash))
return false;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning false does not allow to insert line break before Right token.

return false;
// Sure not to connect after defining keywords.
if (Keywords.isTableGenDefinition(Left))
return true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning true forces to insert space between Left and Right tokens.
Returning false prevents to insert in most cases.

}

TEST_F(FormatTestTableGen, SimpleValue1_MultilineString) {
// verifyFormat does not understand multiline TableGen code-literals
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the verifyFormat with 2 string arguments?

Copy link

github-actions bot commented Feb 14, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

TEST_F(FormatTestTableGen, SimpleValue1_MultilineString) {
// test::messUp does not understand multiline TableGen code-literals.
// We have to give the result and the strings to format manually.
std::string DefWithCode =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::string DefWithCode =
StringRef DefWithCode =

No need to allocate.

@hnakamura5 hnakamura5 merged commit 8c1c94e into llvm:main Feb 16, 2024
4 checks passed
@hnakamura5
Copy link
Contributor Author

Thank you very much!

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