r182449 - Cut-off clang-format analysis.
Daniel Jasper
djasper at google.com
Tue May 21 22:27:42 PDT 2013
Author: djasper
Date: Wed May 22 00:27:42 2013
New Revision: 182449
URL: http://llvm.org/viewvc/llvm-project?rev=182449&view=rev
Log:
Cut-off clang-format analysis.
If clang-format is confronted with long and deeply nested lines (e.g.
complex static initializers or function calls), it can currently try too
hard to find the optimal solution and never finish. The reason is that
the memoization does not work effectively for deeply nested lines.
This patch removes an earlier workaround and instead opts for
accepting a non-optimal solution in rare cases. However, it only does
so only in cases where it would have to analyze an excessive number of
states (currently set to 10000 - the most complex line in Format.cpp
requires ~800 states) so this should not change the behavior in a
relevant way.
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/TokenAnnotator.h
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=182449&r1=182448&r2=182449&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed May 22 00:27:42 2013
@@ -257,6 +257,7 @@ public:
State.ParenLevel = 0;
State.StartOfStringLiteral = 0;
State.StartOfLineLevel = State.ParenLevel;
+ State.IgnoreStackForComparison = false;
// The first token has already been indented and thus consumed.
moveStateToNextToken(State, /*DryRun=*/ false);
@@ -420,6 +421,21 @@ private:
/// levels.
std::vector<ParenState> Stack;
+ /// \brief Ignore the stack of \c ParenStates for state comparison.
+ ///
+ /// In long and deeply nested unwrapped lines, the current algorithm can
+ /// be insufficient for finding the best formatting with a reasonable amount
+ /// of time and memory. Setting this flag will effectively lead to the
+ /// algorithm not analyzing some combinations. However, these combinations
+ /// rarely contain the optimal solution: In short, accepting a higher
+ /// penalty early would need to lead to different values in the \c
+ /// ParenState stack (in an otherwise identical state) and these different
+ /// values would need to lead to a significant amount of avoided penalty
+ /// later.
+ ///
+ /// FIXME: Come up with a better algorithm instead.
+ bool IgnoreStackForComparison;
+
/// \brief Comparison operator to be able to used \c LineState in \c map.
bool operator<(const LineState &Other) const {
if (NextToken != Other.NextToken)
@@ -435,6 +451,8 @@ private:
return StartOfLineLevel < Other.StartOfLineLevel;
if (StartOfStringLiteral != Other.StartOfStringLiteral)
return StartOfStringLiteral < Other.StartOfStringLiteral;
+ if (IgnoreStackForComparison || Other.IgnoreStackForComparison)
+ return false;
return Stack < Other.Stack;
}
};
@@ -713,18 +731,6 @@ private:
AvoidBinPacking = !Style.BinPackParameters;
}
- if (Current.NoMoreTokensOnLevel && Current.FakeLParens.empty()) {
- // This parenthesis was the last token possibly making use of Indent and
- // LastSpace of the next higher ParenLevel. Thus, erase them to achieve
- // better memoization results.
- for (unsigned i = State.Stack.size() - 1; i > 0; --i) {
- State.Stack[i].Indent = 0;
- State.Stack[i].LastSpace = 0;
- if (!State.Stack[i].ForFakeParenthesis)
- break;
- }
- }
-
State.Stack.push_back(ParenState(NewIndent, LastSpace, AvoidBinPacking,
State.Stack.back().NoLineBreak));
++State.ParenLevel;
@@ -917,6 +923,11 @@ private:
}
Queue.pop();
+ // Cut off the analysis of certain solutions if the analysis gets too
+ // complex. See description of IgnoreStackForComparison.
+ if (Count > 10000)
+ Node->State.IgnoreStackForComparison = true;
+
if (!Seen.insert(Node->State).second)
// State already examined with lower penalty.
continue;
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=182449&r1=182448&r2=182449&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed May 22 00:27:42 2013
@@ -160,8 +160,6 @@ private:
if (CurrentToken->Children.empty() ||
!CurrentToken->Children[0].isOneOf(tok::l_paren, tok::l_square))
Left->DefinesFunctionType = false;
- if (CurrentToken->Parent->closesScope())
- CurrentToken->Parent->MatchingParen->NoMoreTokensOnLevel = true;
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
Modified: cfe/trunk/lib/Format/TokenAnnotator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=182449&r1=182448&r2=182449&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.h (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.h Wed May 22 00:27:42 2013
@@ -78,8 +78,7 @@ public:
ParameterCount(0), TotalLength(FormatTok.TokenLength),
BindingStrength(0), SplitPenalty(0), LongestObjCSelectorName(0),
DefinesFunctionType(false), Parent(NULL), FakeRParens(0),
- LastInChainOfCalls(false), PartOfMultiVariableDeclStmt(false),
- NoMoreTokensOnLevel(false) {}
+ LastInChainOfCalls(false), PartOfMultiVariableDeclStmt(false) {}
bool is(tok::TokenKind Kind) const { return FormatTok.Tok.is(Kind); }
@@ -189,19 +188,6 @@ public:
/// Only set if \c Type == \c TT_StartOfName.
bool PartOfMultiVariableDeclStmt;
- /// \brief Set to \c true for "("-tokens if this is the last token other than
- /// ")" in the next higher parenthesis level.
- ///
- /// If this is \c true, no more formatting decisions have to be made on the
- /// next higher parenthesis level, enabling optimizations.
- ///
- /// Example:
- /// \code
- /// aaaaaa(aaaaaa());
- /// ^ // Set to true for this parenthesis.
- /// \endcode
- bool NoMoreTokensOnLevel;
-
/// \brief Returns the previous token ignoring comments.
AnnotatedToken *getPreviousNoneComment() const;
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=182449&r1=182448&r2=182449&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed May 22 00:27:42 2013
@@ -1826,6 +1826,26 @@ TEST_F(FormatTest, MemoizationTests) {
" aaaaa,\n"
" aaaaa))))))))))));",
getLLVMStyleWithColumns(65));
+ verifyFormat(
+ "a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(a(), a), a), a), a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a),\n"
+ " a)",
+ getLLVMStyleWithColumns(65));
// This test takes VERY long when memoization is broken.
FormatStyle OnePerLine = getLLVMStyle();
More information about the cfe-commits
mailing list