[PATCH] [ClangFormat] Add ConstructorInitializerOffset to control initializer list indentation

Daniel Jasper djasper at google.com
Mon Aug 12 23:46:52 PDT 2013


  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



More information about the cfe-commits mailing list