a) I don't have commit access, so you can go ahead and commit the patch when
it's ready.
b) I can't seem to upload full-context diffs because phabricator silently
truncates FormatTest.cpp.
I'll upload a new diff with the proper alphabetical orderings; please see if
my replies address the other issues.
================
Comment at: ../tools/clang/lib/Format/Format.cpp:654
@@ +653,3 @@
+ State.Column = FirstIndent + Style.ConstructorInitializerIndentWidth;
+ State.Stack.back().Indent = State.Column;
+ } else if (Current.Type == TT_CtorInitializerComma) {
----------------
Daniel Jasper wrote:
> I think this is wrong (needs to be State.Column + 2) and unnecessary (gets
> overwritten somewhere at the beginning of moveStateToNextToken()). Try just
> removing this.
Actually, this is necessary for BreakConstructorInitializersBeforeComma. It
only gets overwritten when that option is false.
================
Comment at: ../tools/clang/lib/Format/Format.cpp:656
@@ +655,3 @@
+ } else if (Current.Type == TT_CtorInitializerComma) {
+ assert(Style.BreakConstructorInitializersBeforeComma);
+ State.Column = State.Stack.back().Indent;
----------------
Daniel Jasper wrote:
> Do not assert!!! This is easy to force by user input, e.g. by:
>
> Constructor::Constructor() : a(a) // a
> , b(b) {}
>
> It's fine not to format perfectly in that case, but we shouldn't crash.
Since this whole block is within the `if (Newline)`, `Current` refers to the
first token to emit on that line. Unless we are using the
BreakConstructorInitializersBeforeComma style, it cannot be an initializer list
comma token. So I don't think this can be triggered by user input.
================
Comment at: ../tools/clang/lib/Format/Format.cpp:657
@@ -648,1 +656,3 @@
+ assert(Style.BreakConstructorInitializersBeforeComma);
+ State.Column = State.Stack.back().Indent;
} else {
----------------
Daniel Jasper wrote:
> If you don't assert, this entire if-branch can be removed.
It cannot be removed because the final else block would fall back to indenting
with 4 spaces in cases where `BreakConstructorInitializersBeforeComma = true`
and `ConstructorInitializerIndentWidth = 0`, due to the way it detects
continuations by checking `State.Column == FirstIndent`.
http://llvm-reviews.chandlerc.com/D1360
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
