<div dir="ltr">Short answer: No.<div><br></div><div style>It has been in flux quite a bit, but should now settle somewhat. I want to do one more significant change to it and then I'll try to write up some coarse documentation for it next week.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Feb 1, 2013 at 1:57 PM, David Tweed <span dir="ltr"><<a href="mailto:David.Tweed@arm.com" target="_blank">David.Tweed@arm.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
Out of curiosity is there a description of the cost function that's being assigned (ie, simplified from the code that actually calculates things) available anywhere?<br>
<br>
Regards,<br>
Dave<br>
<div><div class="h5"><br>
-----Original Message-----<br>
From: <a href="mailto:cfe-commits-bounces@cs.uiuc.edu">cfe-commits-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:cfe-commits-bounces@cs.uiuc.edu">cfe-commits-bounces@cs.uiuc.edu</a>] On Behalf Of Daniel Jasper<br>
Sent: 01 February 2013 11:01<br>
To: <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
Subject: r174166 - Revamp of the basic layouting algorithm in clang-format.<br>
<br>
Author: djasper<br>
Date: Fri Feb 1 05:00:45 2013<br>
New Revision: 174166<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=174166&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=174166&view=rev</a><br>
Log:<br>
Revamp of the basic layouting algorithm in clang-format.<br>
<br>
In order to end up with good solutions, clang-format needs to try<br>
"all" combinations of line breaks, evaluate them and select the<br>
best one. Before, we have done this using a DFS with memoization<br>
and cut-off conditions. However, this approach is very limited<br>
as shown by the huge static initializer in the attachment of<br>
<a href="http://llvm.org/PR14959" target="_blank">llvm.org/PR14959</a>.<br>
<br>
Instead, this new implementation uses a variant of Dijkstra's<br>
algorithm to do a prioritized BFS over the solution space.<br>
<br>
Some numbers:<br>
lib/Format/TokenAnnotator.cpp: 1.5s -> 0.15s<br>
Attachment of PR14959: 10min+ (didn't finish) -> 10s<br>
<br>
No functional changes intended.<br>
<br>
Modified:<br>
cfe/trunk/lib/Format/Format.cpp<br>
<br>
Modified: cfe/trunk/lib/Format/Format.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=174166&r1=174165&r2=174166&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=174166&r1=174165&r2=174166&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/Format.cpp (original)<br>
+++ cfe/trunk/lib/Format/Format.cpp Fri Feb 1 05:00:45 2013<br>
@@ -241,44 +241,16 @@ public:<br>
// The first token has already been indented and thus consumed.<br>
moveStateToNextToken(State);<br>
<br>
- // Start iterating at 1 as we have correctly formatted of Token #0 above.<br>
- while (State.NextToken != NULL) {<br>
- if (State.NextToken->Type == TT_ImplicitStringLiteral) {<br>
- // Calculating the column is important for aligning trailing comments.<br>
- // FIXME: This does not seem to happen in conjunction with escaped<br>
- // newlines. If it does, fix!<br>
- State.Column += State.NextToken->FormatTok.WhiteSpaceLength +<br>
- State.NextToken->FormatTok.TokenLength;<br>
- State.NextToken = State.NextToken->Children.empty()<br>
- ? NULL : &State.NextToken->Children[0];<br>
- } else if (Line.Last->TotalLength <= getColumnLimit() - FirstIndent) {<br>
+ // If everything fits on a single line, just put it there.<br>
+ if (Line.Last->TotalLength <= getColumnLimit() - FirstIndent) {<br>
+ while (State.NextToken != NULL) {<br>
addTokenToState(false, false, State);<br>
- } else {<br>
- unsigned NoBreak = calcPenalty(State, false, UINT_MAX);<br>
- unsigned Break = calcPenalty(State, true, NoBreak);<br>
- DEBUG({<br>
- if (Break < NoBreak)<br>
- llvm::errs() << "\n";<br>
- else<br>
- llvm::errs() << " ";<br>
- llvm::errs() << "<";<br>
- DebugPenalty(Break, Break < NoBreak);<br>
- llvm::errs() << "/";<br>
- DebugPenalty(NoBreak, !(Break < NoBreak));<br>
- llvm::errs() << "> ";<br>
- DebugTokenState(*State.NextToken);<br>
- });<br>
- addTokenToState(Break < NoBreak, false, State);<br>
- if (State.NextToken != NULL &&<br>
- State.NextToken->Parent->Type == TT_CtorInitializerColon) {<br>
- if (Style.ConstructorInitializerAllOnOneLineOrOnePerLine &&<br>
- Line.Last->TotalLength > getColumnLimit() - State.Column - 1)<br>
- State.Stack.back().BreakAfterComma = true;<br>
- }<br>
}<br>
+ return State.Column;<br>
}<br>
- DEBUG(llvm::errs() << "\n");<br>
- return State.Column;<br>
+<br>
+ // Find best solution in solution space.<br>
+ return analyzeSolutionSpace(State);<br>
}<br>
<br>
private:<br>
@@ -289,15 +261,6 @@ private:<br>
llvm::errs();<br>
}<br>
<br>
- void DebugPenalty(unsigned Penalty, bool Winner) {<br>
- llvm::errs().changeColor(Winner ? raw_ostream::GREEN : raw_ostream::RED);<br>
- if (Penalty == UINT_MAX)<br>
- llvm::errs() << "MAX";<br>
- else<br>
- llvm::errs() << Penalty;<br>
- llvm::errs().resetColor();<br>
- }<br>
-<br>
struct ParenState {<br>
ParenState(unsigned Indent, unsigned LastSpace, bool AvoidBinPacking)<br>
: Indent(Indent), LastSpace(LastSpace), AssignmentColumn(0),<br>
@@ -421,6 +384,16 @@ private:<br>
assert(State.Stack.size());<br>
unsigned ParenLevel = State.Stack.size() - 1;<br>
<br>
+ if (Current.Type == TT_ImplicitStringLiteral) {<br>
+ State.Column += State.NextToken->FormatTok.WhiteSpaceLength +<br>
+ State.NextToken->FormatTok.TokenLength;<br>
+ if (State.NextToken->Children.empty())<br>
+ State.NextToken = NULL;<br>
+ else<br>
+ State.NextToken = &State.NextToken->Children[0];<br>
+ return;<br>
+ }<br>
+<br>
if (Newline) {<br>
unsigned WhitespaceStartColumn = State.Column;<br>
if (Current.is(tok::r_brace)) {<br>
@@ -604,87 +577,123 @@ private:<br>
return Style.ColumnLimit - (Line.InPPDirective ? 1 : 0);<br>
}<br>
<br>
- /// \brief Calculate the number of lines needed to format the remaining part<br>
- /// of the unwrapped line.<br>
- ///<br>
- /// Assumes the formatting so far has led to<br>
- /// the \c LineSta \p State. If \p NewLine is set, a new line will be<br>
- /// added after the previous token.<br>
+ /// \brief An edge in the solution space starting from the \c LineState and<br>
+ /// inserting a newline dependent on the \c bool.<br>
+ typedef std::pair<bool, const LineState *> Edge;<br>
+<br>
+ /// \brief An item in the prioritized BFS search queue. The \c LineState was<br>
+ /// reached using the \c Edge.<br>
+ typedef std::pair<LineState, Edge> QueueItem;<br>
+<br>
+ /// \brief Analyze the entire solution space starting from \p InitialState.<br>
///<br>
- /// \param StopAt is used for optimization. If we can determine that we'll<br>
- /// definitely need at least \p StopAt additional lines, we already know of a<br>
- /// better solution.<br>
- unsigned calcPenalty(LineState State, bool NewLine, unsigned StopAt) {<br>
- // We are at the end of the unwrapped line, so we don't need any more lines.<br>
- if (State.NextToken == NULL)<br>
+ /// This implements a variant of Dijkstra's algorithm on the graph that spans<br>
+ /// the solution space (\c LineStates are the nodes). The algorithm tries to<br>
+ /// find the shortest path (the one with lowest penalty) from \p InitialState<br>
+ /// to a state where all tokens are placed.<br>
+ unsigned analyzeSolutionSpace(const LineState &InitialState) {<br>
+ // Insert start element into queue.<br>
+ std::multimap<unsigned, QueueItem> Queue;<br>
+ Queue.insert(std::pair<unsigned, QueueItem>(<br>
+ 0, QueueItem(InitialState, Edge(false, NULL))));<br>
+ std::map<LineState, Edge> Seen;<br>
+<br>
+ // While not empty, take first element and follow edges.<br>
+ while (!Queue.empty()) {<br>
+ unsigned Penalty = Queue.begin()->first;<br>
+ QueueItem Item = Queue.begin()->second;<br>
+ if (Item.first.NextToken == NULL)<br>
+ break;<br>
+ Queue.erase(Queue.begin());<br>
+<br>
+ if (Seen.find(Item.first) != Seen.end())<br>
+ continue; // State already examined with lower penalty.<br>
+<br>
+ const LineState &SavedState = Seen.insert(std::pair<LineState, Edge>(<br>
+ Item.first,<br>
+ Edge(Item.second.first, Item.second.second))).first->first;<br>
+<br>
+ addNextStateToQueue(SavedState, Penalty, /*NewLine=*/ false, Queue);<br>
+ addNextStateToQueue(SavedState, Penalty, /*NewLine=*/ true, Queue);<br>
+ }<br>
+<br>
+ if (Queue.empty())<br>
+ // We were unable to find a solution, do nothing.<br>
+ // FIXME: Add diagnostic?<br>
return 0;<br>
<br>
- if (!NewLine && State.NextToken->MustBreakBefore)<br>
- return UINT_MAX;<br>
- if (NewLine && !State.NextToken->CanBreakBefore &&<br>
- !(State.NextToken->is(tok::r_brace) &&<br>
- State.Stack.back().BreakBeforeClosingBrace))<br>
- return UINT_MAX;<br>
- if (!NewLine && State.NextToken->is(tok::r_brace) &&<br>
- State.Stack.back().BreakBeforeClosingBrace)<br>
- return UINT_MAX;<br>
- if (!NewLine && State.NextToken->Parent->is(tok::semi) &&<br>
- State.LineContainsContinuedForLoopSection)<br>
- return UINT_MAX;<br>
- if (!NewLine && State.NextToken->Parent->is(tok::comma) &&<br>
- State.NextToken->Type != TT_LineComment &&<br>
- State.Stack.back().BreakAfterComma)<br>
- return UINT_MAX;<br>
- // Trying to insert a parameter on a new line if there are already more than<br>
- // one parameter on the current line is bin packing.<br>
- if (NewLine && State.NextToken->Parent->is(tok::comma) &&<br>
- State.Stack.back().HasMultiParameterLine &&<br>
- State.Stack.back().AvoidBinPacking)<br>
- return UINT_MAX;<br>
- if (!NewLine && (State.NextToken->Type == TT_CtorInitializerColon ||<br>
- (State.NextToken->Parent->ClosesTemplateDeclaration &&<br>
- State.Stack.size() == 1)))<br>
- return UINT_MAX;<br>
+ // Reconstruct the solution.<br>
+ // FIXME: Add debugging output.<br>
+ Edge *CurrentEdge = &Queue.begin()->second.second;<br>
+ while (CurrentEdge->second != NULL) {<br>
+ LineState CurrentState = *CurrentEdge->second;<br>
+ addTokenToState(CurrentEdge->first, false, CurrentState);<br>
+ CurrentEdge = &Seen[*CurrentEdge->second];<br>
+ }<br>
<br>
- unsigned CurrentPenalty = 0;<br>
- if (NewLine)<br>
- CurrentPenalty += Parameters.PenaltyIndentLevel * State.Stack.size() +<br>
- State.NextToken->SplitPenalty;<br>
+ // Return the column after the last token of the solution.<br>
+ return Queue.begin()->second.first.Column;<br>
+ }<br>
<br>
+ /// \brief Add the following state to the analysis queue \p Queue.<br>
+ ///<br>
+ /// Assume the current state is \p OldState and has been reached with a<br>
+ /// penalty of \p Penalty. Insert a line break if \p NewLine is \c true.<br>
+ void addNextStateToQueue(const LineState &OldState, unsigned Penalty,<br>
+ bool NewLine,<br>
+ std::multimap<unsigned, QueueItem> &Queue) {<br>
+ if (NewLine && !canBreak(OldState))<br>
+ return;<br>
+ if (!NewLine && mustBreak(OldState))<br>
+ return;<br>
+ LineState State(OldState);<br>
+ if (NewLine)<br>
+ Penalty += Parameters.PenaltyIndentLevel * State.Stack.size() +<br>
+ State.NextToken->SplitPenalty;<br>
addTokenToState(NewLine, true, State);<br>
-<br>
- // Exceeding column limit is bad, assign penalty.<br>
if (State.Column > getColumnLimit()) {<br>
unsigned ExcessCharacters = State.Column - getColumnLimit();<br>
- CurrentPenalty += Parameters.PenaltyExcessCharacter * ExcessCharacters;<br>
+ Penalty += Parameters.PenaltyExcessCharacter * ExcessCharacters;<br>
}<br>
+ Queue.insert(std::pair<unsigned, QueueItem>(<br>
+ Penalty, QueueItem(State, Edge(NewLine, &OldState))));<br>
+ }<br>
+<br>
+ /// \brief Returns \c true, if a line break after \p State is allowed.<br>
+ bool canBreak(const LineState &State) {<br>
+ if (!State.NextToken->CanBreakBefore &&<br>
+ !(State.NextToken->is(tok::r_brace) &&<br>
+ State.Stack.back().BreakBeforeClosingBrace))<br>
+ return false;<br>
+ // Trying to insert a parameter on a new line if there are already more than<br>
+ // one parameter on the current line is bin packing.<br>
+ if (State.NextToken->Parent->is(tok::comma) &&<br>
+ State.Stack.back().HasMultiParameterLine &&<br>
+ State.Stack.back().AvoidBinPacking)<br>
+ return false;<br>
+ return true;<br>
+ }<br>
<br>
- if (StopAt <= CurrentPenalty)<br>
- return UINT_MAX;<br>
- StopAt -= CurrentPenalty;<br>
- StateMap::iterator I = Memory.find(State);<br>
- if (I != Memory.end()) {<br>
- // If this state has already been examined, we can safely return the<br>
- // previous result if we<br>
- // - have not hit the optimatization (and thus returned UINT_MAX) OR<br>
- // - are now computing for a smaller or equal StopAt.<br>
- unsigned SavedResult = I->second.first;<br>
- unsigned SavedStopAt = I->second.second;<br>
- if (SavedResult != UINT_MAX)<br>
- return SavedResult + CurrentPenalty;<br>
- else if (StopAt <= SavedStopAt)<br>
- return UINT_MAX;<br>
- }<br>
-<br>
- unsigned NoBreak = calcPenalty(State, false, StopAt);<br>
- unsigned WithBreak = calcPenalty(State, true, std::min(StopAt, NoBreak));<br>
- unsigned Result = std::min(NoBreak, WithBreak);<br>
-<br>
- // We have to store 'Result' without adding 'CurrentPenalty' as the latter<br>
- // can depend on 'NewLine'.<br>
- Memory[State] = std::pair<unsigned, unsigned>(Result, StopAt);<br>
<br>
- return Result == UINT_MAX ? UINT_MAX : Result + CurrentPenalty;<br>
+ /// \brief Returns \c true, if a line break after \p State is mandatory.<br>
+ bool mustBreak(const LineState &State) {<br>
+ if (State.NextToken->MustBreakBefore)<br>
+ return true;<br>
+ if (State.NextToken->is(tok::r_brace) &&<br>
+ State.Stack.back().BreakBeforeClosingBrace)<br>
+ return true;<br>
+ if (State.NextToken->Parent->is(tok::semi) &&<br>
+ State.LineContainsContinuedForLoopSection)<br>
+ return true;<br>
+ if (State.NextToken->Parent->is(tok::comma) &&<br>
+ State.Stack.back().BreakAfterComma &&<br>
+ State.NextToken->Type != TT_LineComment)<br>
+ return true;<br>
+ if ((State.NextToken->Type == TT_CtorInitializerColon ||<br>
+ (State.NextToken->Parent->ClosesTemplateDeclaration &&<br>
+ State.Stack.size() == 1)))<br>
+ return true;<br>
+ return false;<br>
}<br>
<br>
FormatStyle Style;<br>
@@ -694,10 +703,6 @@ private:<br>
const AnnotatedToken &RootToken;<br>
WhitespaceManager &Whitespaces;<br>
<br>
- // A map from an indent state to a pair (Result, Used-StopAt).<br>
- typedef std::map<LineState, std::pair<unsigned, unsigned> > StateMap;<br>
- StateMap Memory;<br>
-<br>
OptimizationParameters Parameters;<br>
};<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
<br>
</div></div>-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.<br>
<br>
</blockquote></div><br></div>