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

[Daniel Jasper]

  Can you elaborate on what exactly you mean by "Phabricator silently truncates FormatTest.cpp"? Seems like a bug that we should fix.


================
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;
----------------
Klemens Baum wrote:
> 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.
Well, I have given you the user input that triggers it.

================
Comment at: ../tools/clang/lib/Format/Format.cpp:657
@@ -648,1 +656,3 @@
+        assert(Style.BreakConstructorInitializersBeforeComma);
+        State.Column = State.Stack.back().Indent;
       } else {
----------------
Klemens Baum wrote:
> 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`.
Ah, I see. The missing context got me ;).

================
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) {
----------------
Klemens Baum wrote:
> 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.
Ah, I see. Then move this there, so it is all in one place. I.e. in moveStateToNextToken(), change the corresponding section to:

    if (Current.Type == TT_CtorInitializerColon) {
      // Indent 2 from the column, so:
      // SomeClass::SomeClass()
      //     : First(...), ...
      //       Next(...)
      //       ^ line up here.
      State.Stack.back().Indent =
          State.Column +
          (Style.BreakConstructorInitializersBeforeComma ? 0 : 2);
      if (Style.ConstructorInitializerAllOnOneLineOrOnePerLine)
        State.Stack.back().AvoidBinPacking = true;
      State.Stack.back().BreakBeforeParameter = false;
    }


http://llvm-reviews.chandlerc.com/D1360