psigillito updated this revision to Diff 403861.
psigillito marked 8 inline comments as done.
psigillito added a comment.
- Merge branch 'main' of https://github.com/llvm/llvm-project
- code review syntax cleanup, sorted vector
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117416/new/
https://reviews.llvm.org/D117416
Files:
clang/lib/Format/FormatToken.h
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3123,6 +3123,46 @@
"label:\n"
" signals.baz();\n"
"}");
+ verifyFormat("private[1];");
+ verifyFormat("testArray[public] = 1;");
+ verifyFormat("public();");
+ verifyFormat("myFunc(public);");
+ verifyFormat("std::vector<int> testVec = {private};");
+ verifyFormat("private.p = 1;");
+ verifyFormat("void function(private...){};");
+ verifyFormat("if (private && public)\n");
+ verifyFormat("private &= true;");
+ verifyFormat("int x = private * public;");
+ verifyFormat("public *= private;");
+ verifyFormat("int x = public + private;");
+ verifyFormat("private++;");
+ verifyFormat("++private;");
+ verifyFormat("public += private;");
+ verifyFormat("public = public - private;");
+ verifyFormat("public->foo();");
+ verifyFormat("private--;");
+ verifyFormat("--private;");
+ verifyFormat("public -= 1;");
+ verifyFormat("if (!private && !public)\n");
+ verifyFormat("public != private;");
+ verifyFormat("int x = public / private;");
+ verifyFormat("public /= 2;");
+ verifyFormat("public = public % 2;");
+ verifyFormat("public %= 2;");
+ verifyFormat("if (public < private)\n");
+ verifyFormat("public << private;");
+ verifyFormat("public <<= private;");
+ verifyFormat("if (public > private)\n");
+ verifyFormat("public >> private;");
+ verifyFormat("public >>= private;");
+ verifyFormat("public ^ private;");
+ verifyFormat("public ^= private;");
+ verifyFormat("public | private;");
+ verifyFormat("public |= private;");
+ verifyFormat("auto x = private ? 1 : 2;");
+ verifyFormat("if (public == private)\n");
+ verifyFormat("void foo(public, private)");
+ verifyFormat("public::foo();");
}
TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2707,14 +2707,28 @@
}
void UnwrappedLineParser::parseAccessSpecifier() {
+ FormatToken *AccessSpecifierCandidate = FormatTok;
nextToken();
// Understand Qt's slots.
if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
nextToken();
+
+ auto COperatorMatch =
+ std::lower_bound(COperatorsFollowingVar.begin(),
+ COperatorsFollowingVar.end(), FormatTok->Tok.getKind());
// Otherwise, we don't know what it is, and we'd better keep the next token.
- if (FormatTok->Tok.is(tok::colon))
+ if (FormatTok->Tok.is(tok::colon)) {
nextToken();
- addUnwrappedLine();
+ addUnwrappedLine();
+ } else if ((COperatorMatch == COperatorsFollowingVar.end() ||
+ *COperatorMatch != FormatTok->Tok.getKind()) &&
+ !FormatTok->Tok.is(tok::coloncolon)) {
+ // Not a variable name nor namespace name.
+ addUnwrappedLine();
+ } else if (AccessSpecifierCandidate) {
+ // Consider the access specifier to be a C identifier.
+ AccessSpecifierCandidate->Tok.setKind(tok::identifier);
+ }
}
void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,10 +100,39 @@
if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
Style.isCSharp())
return 0;
- if (RootToken.isAccessSpecifier(false) ||
- RootToken.isObjCAccessSpecifier() ||
- (RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
- RootToken.Next && RootToken.Next->is(tok::colon))) {
+
+ auto IsAccessModifier = [this, &RootToken]() {
+ if (RootToken.isAccessSpecifier(Style.isCpp()))
+ return true;
+ else if (RootToken.isObjCAccessSpecifier())
+ return true;
+ // Handle Qt signals.
+ else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+ RootToken.Next && RootToken.Next->is(tok::colon)))
+ return true;
+ else if (RootToken.Next &&
+ RootToken.Next->isOneOf(Keywords.kw_slots, Keywords.kw_qslots) &&
+ RootToken.Next->Next && RootToken.Next->Next->is(tok::colon))
+ return true;
+ // Handle malformed access specifier e.g. 'private' without trailing ':'.
+ else if (RootToken.isAccessSpecifier(false)) {
+ if (!RootToken.Next) {
+ return true;
+ }
+ auto COperatorMatch = std::lower_bound(COperatorsFollowingVar.begin(),
+ COperatorsFollowingVar.end(),
+ RootToken.Next->Tok.getKind());
+
+ if ((COperatorMatch == COperatorsFollowingVar.end() ||
+ *COperatorMatch != RootToken.Next->Tok.getKind()) &&
+ !RootToken.Next->Tok.is(tok::coloncolon)) {
+ return true;
+ }
+ }
+ return false;
+ };
+
+ if (IsAccessModifier()) {
// The AccessModifierOffset may be overridden by IndentAccessModifiers,
// in which case we take a negative value of the IndentWidth to simulate
// the upper indent level.
@@ -222,8 +251,9 @@
unsigned Limit =
Style.ColumnLimit == 0 ? UINT_MAX : Style.ColumnLimit - Indent;
- // If we already exceed the column limit, we set 'Limit' to 0. The different
- // tryMerge..() functions can then decide whether to still do merging.
+ // If we already exceed the column limit, we set 'Limit' to 0. The
+ // different tryMerge..() functions can then decide whether to still do
+ // merging.
Limit = TheLine->Last->TotalLength > Limit
? 0
: Limit - TheLine->Last->TotalLength;
@@ -275,7 +305,8 @@
if (Style.AllowShortFunctionsOnASingleLine &
FormatStyle::SFS_InlineOnly) {
// Just checking TheLine->Level != 0 is not enough, because it
- // provokes treating functions inside indented namespaces as short.
+ // provokes treating functions inside indented namespaces as
+ // short.
if ((*I)->Level != 0) {
if (I == B)
return false;
@@ -372,9 +403,9 @@
TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
Style.BraceWrapping.AfterControlStatement ==
FormatStyle::BWACS_MultiLine) {
- // If possible, merge the next line's wrapped left brace with the current
- // line. Otherwise, leave it on the next line, as this is a multi-line
- // control statement.
+ // If possible, merge the next line's wrapped left brace with the
+ // current line. Otherwise, leave it on the next line, as this is a
+ // multi-line control statement.
return (Style.ColumnLimit == 0 ||
TheLine->Last->TotalLength <= Style.ColumnLimit)
? 1
@@ -474,8 +505,8 @@
I[1]->First == I[1]->Last && I + 2 != E &&
I[2]->First->is(tok::r_brace))) {
MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);
- // If we managed to merge the block, count the function header, which is
- // on a separate line.
+ // If we managed to merge the block, count the function header, which
+ // is on a separate line.
if (MergedLines > 0)
++MergedLines;
}
@@ -621,8 +652,8 @@
AnnotatedLine &Line = **I;
// Don't merge ObjC @ keywords and methods.
- // FIXME: If an option to allow short exception handling clauses on a single
- // line is added, change this to not return for @try and friends.
+ // FIXME: If an option to allow short exception handling clauses on a
+ // single line is added, change this to not return for @try and friends.
if (Style.Language != FormatStyle::LK_Java &&
Line.First->isOneOf(tok::at, tok::minus, tok::plus))
return 0;
@@ -673,8 +704,8 @@
FormatStyle::BWACS_Always &&
I + 2 != E && !I[2]->First->is(tok::r_brace))
return 0;
- // FIXME: Consider an option to allow short exception handling clauses on
- // a single line.
+ // FIXME: Consider an option to allow short exception handling clauses
+ // on a single line.
// FIXME: This isn't covered by tests.
// FIXME: For catch, __except, __finally the first token on the line
// is '}', so this isn't correct here.
@@ -717,8 +748,8 @@
if (!nextTwoLinesFitInto(I, Limit))
return 0;
- // Second, check that the next line does not contain any braces - if it
- // does, readability declines when putting it into a single line.
+ // Second, check that the next line does not contain any braces - if
+ // it does, readability declines when putting it into a single line.
if (I[1]->Last->is(TT_LineComment))
return 0;
do {
@@ -868,17 +899,18 @@
///
/// The crucial idea here is that children always get formatted upon
/// encountering the closing brace right after the nested block. Now, if we
- /// are currently trying to keep the "}" on the same line (i.e. \p NewLine is
- /// \c false), the entire block has to be kept on the same line (which is only
- /// possible if it fits on the line, only contains a single statement, etc.
+ /// are currently trying to keep the "}" on the same line (i.e. \p NewLine
+ /// is \c false), the entire block has to be kept on the same line (which is
+ /// only possible if it fits on the line, only contains a single statement,
+ /// etc.
///
- /// If \p NewLine is true, we format the nested block on separate lines, i.e.
- /// break after the "{", format all lines with correct indentation and the put
- /// the closing "}" on yet another new line.
+ /// If \p NewLine is true, we format the nested block on separate lines,
+ /// i.e. break after the "{", format all lines with correct indentation and
+ /// the put the closing "}" on yet another new line.
///
/// This enables us to keep the simple structure of the
- /// \c UnwrappedLineFormatter, where we only have two options for each token:
- /// break or don't break.
+ /// \c UnwrappedLineFormatter, where we only have two options for each
+ /// token: break or don't break.
bool formatChildren(LineState &State, bool NewLine, bool DryRun,
unsigned &Penalty) {
const FormatToken *LBrace = State.NextToken->getPreviousNonComment();
@@ -927,8 +959,8 @@
if (Child->Last->isTrailingComment())
return false;
- // If the child line exceeds the column limit, we wouldn't want to merge it.
- // We add +2 for the trailing " }".
+ // If the child line exceeds the column limit, we wouldn't want to merge
+ // it. We add +2 for the trailing " }".
if (Style.ColumnLimit > 0 &&
Child->Last->TotalLength + State.Column + 2 > Style.ColumnLimit)
return false;
@@ -1065,10 +1097,11 @@
/// Analyze the entire solution space starting from \p InitialState.
///
- /// This implements a variant of Dijkstra's algorithm on the graph that spans
- /// the solution space (\c LineStates are the nodes). The algorithm tries to
- /// find the shortest path (the one with lowest penalty) from \p InitialState
- /// to a state where all tokens are placed. Returns the penalty.
+ /// This implements a variant of Dijkstra's algorithm on the graph that
+ /// spans the solution space (\c LineStates are the nodes). The algorithm
+ /// tries to find the shortest path (the one with lowest penalty) from \p
+ /// InitialState to a state where all tokens are placed. Returns the
+ /// penalty.
///
/// If \p DryRun is \c false, directly applies the changes.
unsigned analyzeSolutionSpace(LineState &InitialState, bool DryRun) {
Index: clang/lib/Format/FormatToken.h
===================================================================
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -122,6 +122,30 @@
TYPE(CSharpGenericTypeConstraintComma) \
TYPE(Unknown)
+/// Sorted Operators that can follow a C variable.
+static const std::vector<clang::tok::TokenKind> COperatorsFollowingVar = {
+ tok::l_square, tok::r_square,
+ tok::l_paren, tok::r_paren,
+ tok::r_brace, tok::period,
+ tok::ellipsis, tok::ampamp,
+ tok::ampequal, tok::star,
+ tok::starequal, tok::plus,
+ tok::plusplus, tok::plusequal,
+ tok::minus, tok::arrow,
+ tok::minusminus, tok::minusequal,
+ tok::exclaim, tok::exclaimequal,
+ tok::slash, tok::slashequal,
+ tok::percent, tok::percentequal,
+ tok::less, tok::lessless,
+ tok::lessequal, tok::lesslessequal,
+ tok::greater, tok::greatergreater,
+ tok::greaterequal, tok::greatergreaterequal,
+ tok::caret, tok::caretequal,
+ tok::pipe, tok::pipepipe,
+ tok::pipeequal, tok::question,
+ tok::semi, tok::equal,
+ tok::equalequal, tok::comma};
+
/// Determines the semantic type of a syntactic token, e.g. whether "<" is a
/// template opener or binary operator.
enum TokenType : uint8_t {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
