A few remaining remarks, but in general, I think this is good to go in. Two
more questions:
a) Do you have commit access or should I commit the patch once it is ready?
b) Could you in general create diffs with more context (ideally the full
file)? That makes reviewing them in phabricator much easier.
Thanks for working on this!
================
Comment at: ../tools/clang/lib/Format/Format.cpp:178
@@ -175,2 +177,3 @@
LLVMStyle.IndentWidth = 2;
+ LLVMStyle.ConstructorInitializerIndentWidth = 4;
LLVMStyle.MaxEmptyLinesToKeep = 1;
----------------
These are sorted alphabetically.
================
Comment at: ../tools/clang/lib/Format/Format.cpp:215
@@ -211,2 +214,3 @@
GoogleStyle.IndentWidth = 2;
+ GoogleStyle.ConstructorInitializerIndentWidth = 4;
GoogleStyle.MaxEmptyLinesToKeep = 1;
----------------
See above.
================
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) {
----------------
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.
================
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;
----------------
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.
================
Comment at: ../tools/clang/lib/Format/Format.cpp:657
@@ -648,1 +656,3 @@
+ assert(Style.BreakConstructorInitializersBeforeComma);
+ State.Column = State.Stack.back().Indent;
} else {
----------------
If you don't assert, this entire if-branch can be removed.
http://llvm-reviews.chandlerc.com/D1360
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
