[PATCH] [ClangFormat] Add ConstructorInitializerOffset to control initializer list indentation
Klemens Baum
klemensbaum at gmail.com
Tue Aug 13 00:45:15 PDT 2013
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
More information about the cfe-commits
mailing list