================
Comment at: lib/Format/FormatToken.h:379
@@ +378,3 @@
+ /// \brief The total length in number of lines.
+ unsigned TotalLength;
+
----------------
Manuel Klimek wrote:
> I'd probably call that LineCount or something else with Line in it then...
Done.
================
Comment at: lib/Format/FormatToken.cpp:39
@@ +38,3 @@
+ return 0;
+ unsigned RemainingColumns = Style.ColumnLimit - State.Stack.back().Indent;
+ const ColumnFormat *Format = getColumnFormat(RemainingColumns);
----------------
Manuel Klimek wrote:
> So 'RemainingColumns' is in characters, but 'Column' below is in "entries"?
Changed to RemainingCharacters.
================
Comment at: lib/Format/FormatToken.cpp:76
@@ +75,3 @@
+ FormatToken *ItemBegin = Token->Next;
+ SmallVector<bool, 6> MustBreakBeforeItem;
+ SmallVector<unsigned, 6> ItemLengthsInLastColumn;
----------------
Manuel Klimek wrote:
> Has the magic number '6' any meaning?
No. Any hint on how to pick these?
================
Comment at: lib/Format/FormatToken.cpp:105
@@ +104,3 @@
+ bool HasRowWithSufficientColumns = false;
+ for (unsigned i = 0, e = ItemLengths.size(), Column = 0; i != e; ++i) {
+ if (MustBreakBeforeItem[i] || Column == Columns) {
----------------
Manuel Klimek wrote:
> Pulling out Columns would make this line idiomatic and easier to read for me.
Done (if I understood correctly).
================
Comment at: lib/Format/FormatToken.cpp:87-88
@@ +86,4 @@
+ : Commas[i];
+ ItemLengths.push_back(ItemEnd->TotalLength - ItemBegin->TotalLength +
+ ItemBegin->CodePointCount);
+ if (ItemEnd->Next && !ItemEnd->Next->HasUnescapedNewline &&
----------------
Manuel Klimek wrote:
> This needs comments - why does subtracting two different token's length give
> anything useful?
Read the documentation of TotalLength .. This is used in several places.
================
Comment at: lib/Format/TokenAnnotator.cpp:275
@@ -271,1 +274,3 @@
+ Left->Role.reset(new CommaSeparatedList(Style));
+ Left->getRoleAs<CommaSeparatedList>()->CommaFound(Current);
} else if (Left->ParameterCount == 0 && Current->isNot(tok::comment)) {
----------------
Manuel Klimek wrote:
> I'd much rather have an interface for Role that contains all the "events"
> that can happen that are significant to roles, and use virtual methods.
What's the benefit? We already need to know that we are dealing with a
CommaSeparatedList in order to create it appropriately..
I agree that there might be more generic concepts once we have more Roles and
understand them.
================
Comment at: lib/Format/FormatToken.cpp:49-67
@@ +48,21 @@
+ unsigned Spaces = State.NextToken->SpacesRequiredBefore;
+ if (Item < Commas.size() && State.NextToken->Previous == Commas[Item]) {
+ if (!State.NextToken->isTrailingComment()) {
+ Spaces += Format->ColumnSizes[Column] - ItemLengths[Item];
+ ++Column;
+ }
+ ++Item;
+ if (Column == Format->Columns) {
+ Column = 0;
+ NewLine = true;
+ }
+ }
+ if (State.NextToken->MustBreakBefore) {
+ Column = 0;
+ NewLine = true;
+ }
+ if (NewLine)
+ Penalty += State.NextToken->SplitPenalty;
+ Spaces -= State.NextToken->SpacesRequiredBefore;
+ Penalty += Indenter->addTokenToState(State, NewLine, DryRun, Spaces);
+ }
----------------
Manuel Klimek wrote:
> All this needs more comments on how this works...
Cleaned up implementation and added comments.
================
Comment at: lib/Format/FormatToken.cpp:85
@@ +84,3 @@
+ const FormatToken *ItemEnd =
+ i == Commas.size() ? Token->MatchingParen->getPreviousNonComment()
+ : Commas[i];
----------------
Manuel Klimek wrote:
> At a first glance this seems inbalanced: wouldn't ItemEnd be one to early in
> for the last item?
This is actually correct. An item ends (for the purpose of column alignment at
the "," or before the "}". Consider
vector<int> a = { a, b, c,
dd, e };
You do not want to take the "}" into account and align "c" relative to it.
However, I discovered a related bug, changed the implementation and added tests.
http://llvm-reviews.chandlerc.com/D1457
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
