[clang-format] Support of TableGen formatting. #76059
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-format Author: H.Nakamura (hnakamura5) ChangesCurrently, TableGen has its language style but the it does not works well. This patch adds total support of TableGen formatting including the support for the code (multi line string), DAG args, bang operators, the cond operator, and the paste operators. OptionsTableGenAllowBreakBeforeInheritColon (Boolean)Allows break before the colon of inheritance. By default this is false. TableGenAllowBreakAfterInheritColon (Boolean)Allows break after the colon of inheritance. By default this is true. TableGenBreakInsideCondOperator (Boolean)Insert the line break after each case of !cond operator. By default this is true. TableGenBreakInsideDAGArgList (Boolean)Insert the line break after each element of DAGArg list. By default this is false. TableGenPreferBreakInsideSquareBracket (Boolean)For whom likes such a style. By default this is false. TableGenSpaceAroundDAGArgColon (Boolean)Insert the space around the colon inside a DAGArg list. By default this is false. TableGenBreakingDAGArgOperators (List of Strings)Works only when TableGenBreakInsideDAGArgList is true. For example the configuration, makes the line break only occurs inside DAGArgs beginning with the specified identifiers 'ins' and 'outs'. AlignConsecutiveTableGenCondOperatorColonsSupports AlignConsecutiveStyle configuration. AlignConsecutiveTableGenBreakingDAGArgColonsSupports AlignConsecutiveStyle configuration. AlignConsecutiveTableGenDefinitionsSupports AlignConsecutiveStyle configuration. Patch is 68.04 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/76059.diff 12 Files Affected:
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 8604dea689f937..30a38aed99866e 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -396,6 +396,36 @@ struct FormatStyle {
/// \version 17
ShortCaseStatementsAlignmentStyle AlignConsecutiveShortCaseStatements;
+ /// Style of aligning consecutive TableGen cond operator colons.
+ /// \code
+ /// !cond(!eq(size, 1) : 1,
+ /// !eq(size, 16): 1,
+ /// true : 0)
+ /// \endcode
+ /// \version 18
+ AlignConsecutiveStyle AlignConsecutiveTableGenCondOperatorColons;
+
+ /// Style of aligning consecutive TableGen DAGArg operator colons.
+ /// Intended to be used with TableGenBreakInsideDAGArgList
+ /// \code
+ /// let dagarg = (ins
+ /// a :$src1,
+ /// aa :$src2,
+ /// aaa:$src3
+ /// )
+ /// \endcode
+ /// \version 18
+ AlignConsecutiveStyle AlignConsecutiveTableGenBreakingDAGArgColons;
+
+ /// Style of aligning consecutive TableGen def colons.
+ /// \code
+ /// def Def : Parent {}
+ /// def DefDef : Parent {}
+ /// def DefDefDef : Parent {}
+ /// \endcode
+ /// \version 18
+ AlignConsecutiveStyle AlignConsecutiveTableGenDefinitions;
+
/// Different styles for aligning escaped newlines.
enum EscapedNewlineAlignmentStyle : int8_t {
/// Don't align escaped newlines.
@@ -3037,6 +3067,7 @@ struct FormatStyle {
bool isProto() const {
return Language == LK_Proto || Language == LK_TextProto;
}
+ bool isTableGen() const { return Language == LK_TableGen; }
/// Language, this format style is targeted at.
/// \version 3.5
@@ -4656,6 +4687,15 @@ struct FormatStyle {
/// \version 8
std::vector<std::string> StatementMacros;
+ /// Tablegen
+ bool TableGenAllowBreakBeforeInheritColon;
+ bool TableGenAllowBreakAfterInheritColon;
+ bool TableGenBreakInsideCondOperator;
+ bool TableGenBreakInsideDAGArgList;
+ bool TableGenPreferBreakInsideSquareBracket;
+ bool TableGenSpaceAroundDAGArgColon;
+ std::vector<std::string> TableGenBreakingDAGArgOperators;
+
/// The number of columns used for tab stops.
/// \version 3.7
unsigned TabWidth;
@@ -4753,6 +4793,13 @@ struct FormatStyle {
AlignConsecutiveMacros == R.AlignConsecutiveMacros &&
AlignConsecutiveShortCaseStatements ==
R.AlignConsecutiveShortCaseStatements &&
+ AlignConsecutiveTableGenCondOperatorColons ==
+ R.AlignConsecutiveTableGenCondOperatorColons &&
+ AlignConsecutiveTableGenBreakingDAGArgColons ==
+ R.AlignConsecutiveTableGenBreakingDAGArgColons &&
+ AlignConsecutiveTableGenDefinitions ==
+ R.AlignConsecutiveTableGenDefinitions &&
+ AlignConsecutiveMacros == R.AlignConsecutiveMacros &&
AlignEscapedNewlines == R.AlignEscapedNewlines &&
AlignOperands == R.AlignOperands &&
AlignTrailingComments == R.AlignTrailingComments &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index bd319f21b05f86..176dc7744a5576 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -800,6 +800,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))) {
@@ -1229,7 +1230,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;
@@ -1257,6 +1258,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 must be aligned in the similar
+ // way as function call above.
+ if (Current.is(tok::r_paren) && State.Stack.size() > 1 &&
+ (Current.isOneOf(TT_TableGenDAGArgCloser, TT_TableGenParamAngleCloser))) {
+ 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 &&
@@ -1593,6 +1600,9 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
State.StartOfStringLiteral = State.Column + 1;
if (Current.is(TT_CSharpStringLiteral) && State.StartOfStringLiteral == 0) {
State.StartOfStringLiteral = State.Column + 1;
+ } else if (Current.is(TT_TableGenMultiLineString) &&
+ State.StartOfStringLiteral == 0) {
+ State.StartOfStringLiteral = State.Column + 1;
} else if (Current.isStringLiteral() && State.StartOfStringLiteral == 0) {
State.StartOfStringLiteral = State.Column;
} else if (!Current.isOneOf(tok::comment, tok::identifier, tok::hash) &&
@@ -1672,7 +1682,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->isNot(TT_TableGenDAGArgListComma))) {
NewParenState.Indent = std::max(
std::max(State.Column, NewParenState.Indent), CurrentState.LastSpace);
}
@@ -1915,6 +1927,8 @@ 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_TableGenParamAngleCloser) ||
+ State.NextToken->is(TT_TableGenListCloser) ||
(Current.is(tok::greater) && Current.is(TT_DictLiteral)))) {
State.Stack.pop_back();
}
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 28271181e07d0c..9ba3d5dbd530e7 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -926,6 +926,12 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("AlignConsecutiveMacros", Style.AlignConsecutiveMacros);
IO.mapOptional("AlignConsecutiveShortCaseStatements",
Style.AlignConsecutiveShortCaseStatements);
+ IO.mapOptional("AlignConsecutiveTableGenCondOperatorColons",
+ Style.AlignConsecutiveTableGenCondOperatorColons);
+ IO.mapOptional("AlignConsecutiveTableGenBreakingDAGArgColons",
+ Style.AlignConsecutiveTableGenBreakingDAGArgColons);
+ IO.mapOptional("AlignConsecutiveTableGenDefinitions",
+ Style.AlignConsecutiveTableGenDefinitions);
IO.mapOptional("AlignEscapedNewlines", Style.AlignEscapedNewlines);
IO.mapOptional("AlignOperands", Style.AlignOperands);
IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
@@ -1124,6 +1130,20 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("StatementAttributeLikeMacros",
Style.StatementAttributeLikeMacros);
IO.mapOptional("StatementMacros", Style.StatementMacros);
+ IO.mapOptional("TableGenAllowBreakAfterInheritColon",
+ Style.TableGenAllowBreakAfterInheritColon);
+ IO.mapOptional("TableGenAllowBreakBeforeInheritColon",
+ Style.TableGenAllowBreakBeforeInheritColon);
+ IO.mapOptional("TableGenBreakInsideCondOperator",
+ Style.TableGenBreakInsideCondOperator);
+ IO.mapOptional("TableGenBreakInsideDAGArgList",
+ Style.TableGenBreakInsideDAGArgList);
+ IO.mapOptional("TableGenPreferBreakInsideSquareBracket",
+ Style.TableGenPreferBreakInsideSquareBracket);
+ IO.mapOptional("TableGenSpaceAroundDAGArgColon",
+ Style.TableGenSpaceAroundDAGArgColon);
+ IO.mapOptional("TableGenBreakingDAGArgOperators",
+ Style.TableGenBreakingDAGArgOperators);
IO.mapOptional("TabWidth", Style.TabWidth);
IO.mapOptional("TypeNames", Style.TypeNames);
IO.mapOptional("TypenameMacros", Style.TypenameMacros);
@@ -1437,6 +1457,9 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.AlignConsecutiveDeclarations = {};
LLVMStyle.AlignConsecutiveMacros = {};
LLVMStyle.AlignConsecutiveShortCaseStatements = {};
+ LLVMStyle.AlignConsecutiveTableGenBreakingDAGArgColons = {};
+ LLVMStyle.AlignConsecutiveTableGenCondOperatorColons = {};
+ LLVMStyle.AlignConsecutiveTableGenDefinitions = {};
LLVMStyle.AlignTrailingComments = {};
LLVMStyle.AlignTrailingComments.Kind = FormatStyle::TCAS_Always;
LLVMStyle.AlignTrailingComments.OverEmptyLines = 0;
@@ -1585,6 +1608,12 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.StatementAttributeLikeMacros.push_back("Q_EMIT");
LLVMStyle.StatementMacros.push_back("Q_UNUSED");
LLVMStyle.StatementMacros.push_back("QT_REQUIRE_VERSION");
+ LLVMStyle.TableGenAllowBreakAfterInheritColon = true;
+ LLVMStyle.TableGenAllowBreakBeforeInheritColon = false;
+ LLVMStyle.TableGenBreakInsideCondOperator = true;
+ LLVMStyle.TableGenBreakInsideDAGArgList = false;
+ LLVMStyle.TableGenPreferBreakInsideSquareBracket = false;
+ LLVMStyle.TableGenSpaceAroundDAGArgColon = false;
LLVMStyle.TabWidth = 8;
LLVMStyle.UseTab = FormatStyle::UT_Never;
LLVMStyle.VerilogBreakBetweenInstancePorts = true;
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 3f9664f8f78a3e..bd16567233bb22 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -148,6 +148,23 @@ namespace format {
TYPE(StructLBrace) \
TYPE(StructRBrace) \
TYPE(StructuredBindingLSquare) \
+ TYPE(TableGenBangOperator) \
+ TYPE(TableGenCondOperator) \
+ TYPE(TableGenCondOperatorColon) \
+ TYPE(TableGenCondOperatorComma) \
+ TYPE(TableGenDAGArgCloser) \
+ TYPE(TableGenDAGArgListColon) \
+ TYPE(TableGenDAGArgListColonToAlign) \
+ TYPE(TableGenDAGArgListComma) \
+ TYPE(TableGenDAGArgOpener) \
+ TYPE(TableGenDAGArgOperatorID) \
+ TYPE(TableGenListCloser) \
+ TYPE(TableGenListOpener) \
+ TYPE(TableGenMultiLineString) \
+ TYPE(TableGenParamAngleOpener) \
+ TYPE(TableGenParamAngleCloser) \
+ TYPE(TableGenTrailingPasteOperator) \
+ TYPE(TableGenValueSuffix) \
TYPE(TemplateCloser) \
TYPE(TemplateOpener) \
TYPE(TemplateString) \
@@ -671,6 +688,8 @@ struct FormatToken {
return true;
if (is(TT_DictLiteral) && is(tok::less))
return true;
+ if (is(TT_TableGenParamAngleOpener))
+ return true;
return isOneOf(tok::l_paren, tok::l_brace, tok::l_square,
TT_TemplateOpener);
}
@@ -681,6 +700,10 @@ struct FormatToken {
return true;
if (is(TT_DictLiteral) && is(tok::greater))
return true;
+ if (is(TT_TableGenParamAngleCloser))
+ return true;
+ if (is(TT_TableGenListCloser))
+ return true;
return isOneOf(tok::r_paren, tok::r_brace, tok::r_square,
TT_TemplateCloser);
}
@@ -1202,6 +1225,21 @@ struct AdditionalKeywords {
kw_verilogHashHash = &IdentTable.get("##");
kw_apostrophe = &IdentTable.get("\'");
+ // TableGen keywords
+ kw_bit = &IdentTable.get("bit");
+ kw_bits = &IdentTable.get("bits");
+ kw_code = &IdentTable.get("code");
+ kw_dag = &IdentTable.get("dag");
+ kw_def = &IdentTable.get("def");
+ kw_defm = &IdentTable.get("defm");
+ kw_defset = &IdentTable.get("defset");
+ kw_defvar = &IdentTable.get("defvar");
+ kw_dump = &IdentTable.get("dump");
+ kw_include = &IdentTable.get("include");
+ kw_list = &IdentTable.get("list");
+ kw_multiclass = &IdentTable.get("multiclass");
+ kw_then = &IdentTable.get("then");
+
// Keep this at the end of the constructor to make sure everything here
// is
// already initialized.
@@ -1294,6 +1332,27 @@ struct AdditionalKeywords {
kw_wildcard, kw_wire,
kw_with, kw_wor,
kw_verilogHash, kw_verilogHashHash});
+
+ TableGenExtraKeywords = std::unordered_set<IdentifierInfo *>({
+ kw_assert,
+ kw_bit,
+ kw_bits,
+ kw_code,
+ kw_dag,
+ kw_def,
+ kw_defm,
+ kw_defset,
+ kw_defvar,
+ kw_dump,
+ kw_foreach,
+ kw_in,
+ kw_include,
+ kw_let,
+ kw_list,
+ kw_multiclass,
+ kw_string,
+ kw_then,
+ });
}
// Context sensitive keywords.
@@ -1539,6 +1598,21 @@ struct AdditionalKeywords {
// Symbols in Verilog that don't exist in C++.
IdentifierInfo *kw_apostrophe;
+ // TableGen keywords
+ IdentifierInfo *kw_bit;
+ IdentifierInfo *kw_bits;
+ IdentifierInfo *kw_code;
+ IdentifierInfo *kw_dag;
+ IdentifierInfo *kw_def;
+ IdentifierInfo *kw_defm;
+ IdentifierInfo *kw_defset;
+ IdentifierInfo *kw_defvar;
+ IdentifierInfo *kw_dump;
+ IdentifierInfo *kw_include;
+ IdentifierInfo *kw_list;
+ IdentifierInfo *kw_multiclass;
+ IdentifierInfo *kw_then;
+
/// Returns \c true if \p Tok is a keyword or an identifier.
bool isWordLike(const FormatToken &Tok) const {
// getIdentifierinfo returns non-null for keywords as well as identifiers.
@@ -1811,6 +1885,27 @@ struct AdditionalKeywords {
}
}
+ bool isTableGenDefinition(const FormatToken &Tok) const {
+ return Tok.isOneOf(kw_def, kw_defm, kw_defset, kw_defvar, kw_multiclass,
+ kw_let, tok::kw_class);
+ }
+
+ bool isTableGenKeyword(const FormatToken &Tok) const {
+ switch (Tok.Tok.getKind()) {
+ case tok::kw_class:
+ case tok::kw_else:
+ case tok::kw_false:
+ case tok::kw_if:
+ case tok::kw_int:
+ case tok::kw_true:
+ return true;
+ default:
+ return Tok.is(tok::identifier) &&
+ TableGenExtraKeywords.find(Tok.Tok.getIdentifierInfo()) !=
+ TableGenExtraKeywords.end();
+ }
+ }
+
private:
/// The JavaScript keywords beyond the C++ keyword set.
std::unordered_set<IdentifierInfo *> JsExtraKeywords;
@@ -1820,6 +1915,9 @@ struct AdditionalKeywords {
/// The Verilog keywords beyond the C++ keyword set.
std::unordered_set<IdentifierInfo *> VerilogExtraKeywords;
+
+ /// The TableGen keywords beyond the C++ keyword set.
+ std::unordered_set<IdentifierInfo *> TableGenExtraKeywords;
};
inline bool isLineComment(const FormatToken &FormatTok) {
diff --git a/clang/lib/Format/FormatTokenLexer.cpp b/clang/lib/Format/FormatTokenLexer.cpp
index 61430282c6f88c..8c2961ff1cbefa 100644
--- a/clang/lib/Format/FormatTokenLexer.cpp
+++ b/clang/lib/Format/FormatTokenLexer.cpp
@@ -93,6 +93,10 @@ ArrayRef<FormatToken *> FormatTokenLexer::lex() {
// string literals are correctly identified.
handleCSharpVerbatimAndInterpolatedStrings();
}
+ if (Style.isTableGen()) {
+ handleTableGenMultilineString();
+ handleTableGenNumericLikeIdentifier();
+ }
if (Tokens.back()->NewlinesBefore > 0 || Tokens.back()->IsMultiline)
FirstInLineIndex = Tokens.size() - 1;
} while (Tokens.back()->isNot(tok::eof));
@@ -272,6 +276,38 @@ void FormatTokenLexer::tryMergePreviousTokens() {
return;
}
}
+ if (Style.isTableGen()) {
+ if (tryMergeTokens({tok::l_square, tok::l_brace},
+ TT_TableGenMultiLineString)) {
+ Tokens.back()->Tok.setKind(tok::string_literal);
+ return;
+ }
+ if (Tokens.size() > 1 && Tokens.end()[-2]->is(tok::exclaim)) {
+ if (Tokens.back()->is(tok::identifier) ||
+ (Tokens.back()->is(tok::kw_if) && Tokens.back())) {
+ }
+ }
+ if (tryMergeTokens({tok::exclaim, tok::identifier},
+ TT_TableGenBangOperator)) {
+ Tokens.back()->Tok.setKind(tok::identifier);
+ if (Tokens.back()->TokenText == "!cond")
+ Tokens.back()->setType(TT_TableGenCondOperator);
+ return;
+ }
+ if (tryMergeTokens({tok::exclaim, tok::kw_if}, TT_TableGenBangOperator)) {
+ // Here, "! if" becomes "!if". ! captures if even if the space exists.
+ // That is surely is a only one possibility in TableGen's syntax.
+ return;
+ }
+ if (tryMergeTokens({tok::plus, tok::numeric_constant}, TT_Unknown)) {
+ Tokens.back()->Tok.setKind(tok::numeric_constant);
+ return;
+ }
+ if (tryMergeTokens({tok::minus, tok::numeric_constant}, TT_Unknown)) {
+ Tokens.back()->Tok.setKind(tok::numeric_constant);
+ return;
+ }
+ }
}
bool FormatTokenLexer::tryMergeNSStringLiteral() {
@@ -763,6 +799,108 @@ void FormatTokenLexer::handleCSharpVerbatimAndInterpolatedStrings() {
resetLexer(SourceMgr.getFileOffset(Lex->getSourceLocation(Offset + 1)));
}
+void FormatTokenLexer::handleTableGenMultilineString() {
+ FormatToken *MultiLineString = Tokens.back();
+ if (MultiLineString->isNot(TT_TableGenMultiLineString)) {
+ // Multi line string starts with [{
+ return;
+ }
+ bool PrevIsRBrace = false;
+ const char *FirstBreak = nullptr;
+ const char *LastBreak = nullptr;
+ const char *Begin = MultiLineString->TokenText.begin();
+ for (const char *Current = Begin, *End = Lex->getBuffer().end();
+ Current != End; ++Current) {
+ if (*Current == ']' && PrevIsRBrace) {
+ // }] is the end of multi line string.
+ if (!FirstBreak)
+ FirstBreak = Current;
+ MultiLineString->TokenText = StringRef(Begin, Current - Begin + 1);
+ MultiLineString->ColumnWidth = encoding::columnWidthWithTabs(
+ StringRef(Begin, FirstBreak - Begin + 1),
+ MultiLineString->OriginalColumn, Style.TabWidth, Encoding);
+ if (LastBreak) {
+ MultiLineString->LastLineColumnWidth = encoding::columnWidthWithTabs(
+ StringRef(LastBreak + 1, Current - LastBreak),
+ MultiLineString->OriginalColumn, Style.TabWidth, Encoding);
+ }
+ resetLexer(SourceMgr.getFileOffset(Lex->getSourceLocation(Current + 1)));
+ return;
+ }
+ PrevIsRBrace = false;
+ if (*Current == '\n') {
+ MultiLineString->IsMultiline = true;
+ // Assure LastBreak is not equal to FirstBreak.
+ if (!FirstBreak)
+ FirstBreak = Current;
+ LastBreak = Current;
+ continue;
+ }
+ if (*Current == '}') {
+ // Memorize '}'. If ne...
[truncated]
|
|
The failure in Check code formatting seems to be caused by the commit (replacing to clang-format style) I avoid this by using clang-format binary from main branch in my local. |
You can ignore it if running the in-tree clang-format is clean, e.g.: |
|
Thank you for your advice. I have checked the message "clang-format did not modify any files" in my local. |
Currently, TableGen has its language style but the it does not works well. This patch adds total support of TableGen formatting including the support for the code (multi line string), DAG args, bang operators, the cond operator, and the paste operators.
hnakamura5
force-pushed
the
tablegen_format
branch
from
562f792
to
b0080a4
Compare
There was a problem hiding this comment.
Hi, this is quite a big patch. I had a very quick look and noticed a few nit-picky thingy, mostly about formatting or naming and whatnot, mostly minor things.
However since I've never used tablegen, I'm not sure if I can actually review any of the logic in here, but I'll try to have another look later
clang/include/clang/Format/Format.h
Outdated
| R.AlignConsecutiveTableGenBreakingDAGArgColons && | ||
| AlignConsecutiveTableGenDefinitions == | ||
| R.AlignConsecutiveTableGenDefinitions && | ||
| AlignConsecutiveMacros == R.AlignConsecutiveMacros && |
There was a problem hiding this comment.
Small copy paste error, I'm assuming, AlignConsecutiveMacros has already been checked.
Also, these options should be alphabetical in order
There was a problem hiding this comment.
Your edits in Format.h won't show up in the documentation until after you've run the script in clang/docs/tools/dump_format_style.py
There was a problem hiding this comment.
Thank you for the information. I added the document.
clang/lib/Format/Format.cpp
Outdated
| IO.mapOptional("TableGenAllowBreakAfterInheritColon", | ||
| Style.TableGenAllowBreakAfterInheritColon); | ||
| IO.mapOptional("TableGenAllowBreakBeforeInheritColon", | ||
| Style.TableGenAllowBreakBeforeInheritColon); | ||
| IO.mapOptional("TableGenBreakInsideCondOperator", | ||
| Style.TableGenBreakInsideCondOperator); | ||
| IO.mapOptional("TableGenBreakInsideDAGArgList", | ||
| Style.TableGenBreakInsideDAGArgList); | ||
| IO.mapOptional("TableGenPreferBreakInsideSquareBracket", | ||
| Style.TableGenPreferBreakInsideSquareBracket); | ||
| IO.mapOptional("TableGenSpaceAroundDAGArgColon", | ||
| Style.TableGenSpaceAroundDAGArgColon); | ||
| IO.mapOptional("TableGenBreakingDAGArgOperators", | ||
| Style.TableGenBreakingDAGArgOperators); |
There was a problem hiding this comment.
I'm a little confused by these undocumented style options?
| if (Tokens.size() > 1 && Tokens.end()[-2]->is(tok::exclaim)) { | ||
| if (Tokens.back()->is(tok::identifier) || | ||
| (Tokens.back()->is(tok::kw_if) && Tokens.back())) { | ||
| } | ||
| } |
There was a problem hiding this comment.
This if statement does nothing, or am I misunderstanding it?
| @@ -111,6 +111,9 @@ const tooling::Replacements &WhitespaceManager::generateReplacements() { | |||
| alignConsecutiveDeclarations(); | |||
| alignConsecutiveBitFields(); | |||
| alignConsecutiveAssignments(); | |||
| alignConsecutiveTableGenCondOperatorColons(); | |||
| AlignConsecutiveTableGenBreakingDAGArgColons(); | |||
There was a problem hiding this comment.
This one has different capitalization from all the others
| alignConsecutiveTableGenCondOperatorColons(); | ||
| AlignConsecutiveTableGenBreakingDAGArgColons(); | ||
| alignConsecutiveTableGenDefinition(); |
There was a problem hiding this comment.
Shouldn't these replacements be guarded behind an isTableGen? At the very least, alignConsecutiveTableGenDefinition aligns on TT_InheritanceColon and that can appear in non-tablegen code too
| @@ -40,6 +40,13 @@ class FormatTestTableGen : public ::testing::Test { | |||
| EXPECT_EQ(Code.str(), format(Code)) << "Expected code is not stable"; | |||
| EXPECT_EQ(Code.str(), format(test::messUp(Code))); | |||
| } | |||
|
|
|||
| static void verifyFormat(llvm::StringRef Code, const FormatStyle &Style) { | |||
There was a problem hiding this comment.
It seems like this file was never brought up to date with all the other tests when f8d10d5 happened
There was a problem hiding this comment.
I have not noticed that. Thank you. I can also fix this. But now I am planing whether (and how) to split this growing pull request. A kind of refactoring may be better done after that.
clang/include/clang/Format/Format.h
Outdated
| /// def DefDefDef : Parent {} | ||
| /// \endcode | ||
| /// \version 18 | ||
| AlignConsecutiveStyle AlignConsecutiveTableGenDefinitions; |
There was a problem hiding this comment.
Couldn't you reuse AlignConsecutiveDeclarations and handle table gen there?
There was a problem hiding this comment.
The option AlignConsecutiveDeclarations aligns the identifier that is declared.
This AlignConsecutiveTableGenDefinitions option aligns the inheritance colon of def, that is the style often seen in TableGen file formatted by hand.
The name seems confusing. I changed the name to clarify it aligns colon.
| /// ) | ||
| /// \endcode | ||
| /// \version 18 | ||
| AlignConsecutiveStyle AlignConsecutiveTableGenBreakingDAGArgColons; |
There was a problem hiding this comment.
Please sort alphabetically.
clang/include/clang/Format/Format.h
Outdated
| @@ -4656,6 +4687,15 @@ struct FormatStyle { | |||
| /// \version 8 | |||
| std::vector<std::string> StatementMacros; | |||
|
|
|||
| /// Tablegen | |||
There was a problem hiding this comment.
We need a bit more documentation here.
clang/include/clang/Format/Format.h
Outdated
| bool TableGenAllowBreakBeforeInheritColon; | ||
| bool TableGenAllowBreakAfterInheritColon; |
There was a problem hiding this comment.
Do you need both options to be enabled at the same time? Otherwise BreakInheritanceList should already be the option to check.
There was a problem hiding this comment.
Thank you for the information. I removed these options. Instead BreakInheritanceList works well. There happens some changes, but seems acceptable.
clang/lib/Format/FormatToken.h
Outdated
| if (is(TT_TableGenParamAngleCloser)) | ||
| return true; | ||
| if (is(TT_TableGenListCloser)) | ||
| return true; | ||
| return isOneOf(tok::r_paren, tok::r_brace, tok::r_square, | ||
| TT_TemplateCloser); |
There was a problem hiding this comment.
| if (is(TT_TableGenParamAngleCloser)) | |
| return true; | |
| if (is(TT_TableGenListCloser)) | |
| return true; | |
| return isOneOf(tok::r_paren, tok::r_brace, tok::r_square, | |
| TT_TemplateCloser); | |
| return isOneOf(tok::r_paren, tok::r_brace, tok::r_square, | |
| TT_TemplateCloser, TT_TableGenParamAngleCloser, TT_TableGenListCloser); |
| auto Result = annotate(("def X { let V = " + Code + "; }").str(), Style); | ||
| return decltype(Result){Result.begin() + 6, Result.end() - 3}; | ||
| }; | ||
| // Both of bang/cond operators |
There was a problem hiding this comment.
| // Both of bang/cond operators | |
| // Both of bang/cond operators. |
- NOT fixed: Make FormatTestTableGen.cpp up to date to llvm@f8d10d5. I am planning to fix it in other pull request.
hnakamura5
force-pushed
the
tablegen_format
branch
from
9e645d9
to
e579564
Compare
|
@rymiel @HazardyKnusperkeks Now I really understand I should split this pull request into some parts. At first it is large and continue growing by adding documents.
I'm not sure this is good plan. They may be complicated. In addition, I do not know the appropriate way to split pull request after I made one. Is it enough to refer each other, and abort this at last? |
Just create a pull request per feature/bug. You can sill use this for one of them, or abandon this. If some of them build on each other there is, as far as I know, no good way. The few times I came to this I created multiple PRs and the top most commit(s) is/are part of multiple PRs and I mentioned this. Then we have to make sure that they are merged correctly. With multiple local branches you can still work on all of them in parallel. My experience (here and at work) is, that smaller changes are easier to check and you get a faster response. Because 50~150 lines changes I can just review and move on. A 500+ change review will more likely be postponed. |
|
Thanks to the advises, I begin to split this into several parts. Made the keywords part in #77477 . |
Add TableGen keywords to the additional keyword list of the formatter. This pull request is the splited part from #76059 .
|
Multi line string part #78032 |
|
Numeric like identifiers part #78571 |
|
unwrapped line parser for statements part #78846 . |
|
Numeric like identifiers part #78571 |
|
bang operators and numeric literals part #78996 . |
Add TableGen keywords to the additional keyword list of the formatter. This pull request is the splited part from llvm#76059 .
|
Alignment option for cond operator: #82878. |
|
Alignment option for definitions: #83008. |
|
Break options for DAGArg: #83149. |
|
Alignment option for DAGArg: #86150 |
|
All the split parts of this PR is merged. Thank you! |
Currently, TableGen has its language style but the it does not works well. This patch adds total support of TableGen formatting including the support for the code (multi line string), DAG args, bang operators, the cond operator, and the paste operators.
Options
TableGenAllowBreakBeforeInheritColon (Boolean)
Allows break before the colon of inheritance.
By default this is false.
TableGenAllowBreakAfterInheritColon (Boolean)
Allows break after the colon of inheritance.
By default this is true.
TableGenBreakInsideCondOperator (Boolean)
Insert the line break after each case of !cond operator.
By default this is true.
TableGenBreakInsideDAGArgList (Boolean)
Insert the line break after each element of DAGArg list.
By default this is false.
TableGenPreferBreakInsideSquareBracket (Boolean)
For whom likes such a style.
By default this is false.
TableGenSpaceAroundDAGArgColon (Boolean)
Insert the space around the colon inside a DAGArg list.
By default this is false.
TableGenBreakingDAGArgOperators (List of Strings)
Works only when TableGenBreakInsideDAGArgList is true.
The list needs to be consists of identifiers.
If any identifier is specified, this limits the effect of TableGenBreakInsideDAGArgList only on DAGArgs beginning with the specified identifiers.
For example the configuration,
makes the line break only occurs inside DAGArgs beginning with the specified identifiers 'ins' and 'outs'.
AlignConsecutiveTableGenCondOperatorColons
Supports AlignConsecutiveStyle configuration.
Align the colons inside !cond operators.
AlignConsecutiveTableGenBreakingDAGArgColons
Supports AlignConsecutiveStyle configuration.
This works only when TableGenBreakInsideDAGArgList is true and the DAGArg is not excepted by TableGenBreakingDAGArgOperators's effect.
Align the colon inside DAGArg which have line break inside.
AlignConsecutiveTableGenDefinitions
Supports AlignConsecutiveStyle configuration.
This aligns the inherits colons of consecutive definitions.