[PATCH] D60361: [clang-format] [PR41407] Constructor initializer list indented incorrectly

[MyDeveloperDay via Phabricator via llvm-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: klimek, djasper, reuk, owenpan.
MyDeveloperDay added a project: clang-tools-extra.

When using ConstructorInitializerIndentWidth with a value of 0, after the first initializer the members would be incorrectly indented

C++ code to be formatted:
Foo::Foo() :
foo(0),
bar(1) {
}

Incorrectly formatted to:
Foo::Foo() :
foo(0),

  bar(1) {

}

This fix addresses this.

I also introduce the VERIFYFORMAT() macro into the tests to allow for the line number of the failure to be printed when the tests fail, when developing tests for this, I found it hard to locate the exact line each time, passing __LINE__ down via the VERIFYFORMAT() macro means you get a clear indication of where its breaking  (if someone know a non macro trick for doing this I'd be happy to know it)

Moving forward I'd like to use this in new tests


https://reviews.llvm.org/D60361

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -70,10 +70,11 @@
   }
 
   void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
-                    const FormatStyle &Style = getLLVMStyle()) {
+                    const FormatStyle &Style = getLLVMStyle(),
+                    int line = __LINE__) {
     EXPECT_EQ(Expected.str(), format(Expected, Style))
-        << "Expected code is not stable";
-    EXPECT_EQ(Expected.str(), format(Code, Style));
+        << "Expected code is not stable on line " << line;
+    EXPECT_EQ(Expected.str(), format(Code, Style)) << " on line " << line;
     if (Style.Language == FormatStyle::LK_Cpp) {
       // Objective-C++ is a superset of C++, so everything checked for C++
       // needs to be checked for Objective-C++ as well.
@@ -88,6 +89,9 @@
     verifyFormat(Code, test::messUp(Code), Style);
   }
 
+#define VERIFYFORMAT(code, style)                                              \
+  verifyFormat(code, test::messUp(code), style, __LINE__)
+
   void verifyIncompleteFormat(llvm::StringRef Code,
                               const FormatStyle &Style = getLLVMStyle()) {
     EXPECT_EQ(Code.str(),
@@ -11946,6 +11950,21 @@
       "bool smaller = 1 < bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(\n"
       "                       aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);",
       Style);
+
+  Style = getLLVMStyle();
+  Style.ColumnLimit = 0;
+  Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon;
+  Style.ConstructorInitializerIndentWidth = 2;
+  Style.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  Style.AllowAllConstructorInitializersOnNextLine = false;
+  VERIFYFORMAT("SomeClass::Constructor() :\n  a(a),\n  b(b),\n  c(c) {}",
+               Style);
+
+  Style.ConstructorInitializerIndentWidth = 1;
+  VERIFYFORMAT("SomeClass::Constructor() :\n a(a),\n b(b),\n c(c) {}", Style);
+
+  Style.ConstructorInitializerIndentWidth = 0;
+  VERIFYFORMAT("SomeClass::Constructor() :\na(a),\nb(b),\nc(c) {}", Style);
 }
 
 TEST_F(FormatTest, BreakConstructorInitializersBeforeComma) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===================================================================
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1056,6 +1056,13 @@
     return ContinuationIndent;
   if (Current.is(TT_ProtoExtensionLSquare))
     return State.Stack.back().Indent;
+  // If the previous token is a Constructor initializer comma, and the
+  // ConstructorInitializerIndentWidth is 0 then the Indent will equal the
+  // FirstIndent, and it would default to the ContinuationIndentWidth and not
+  // the ConstructorInitializerIndentWidth width.
+  if (State.Stack.back().Indent == State.FirstIndent && PreviousNonComment &&
+      PreviousNonComment->is(TT_CtorInitializerComma))
+    return State.FirstIndent + Style.ConstructorInitializerIndentWidth;
   if (State.Stack.back().Indent == State.FirstIndent && PreviousNonComment &&
       PreviousNonComment->isNot(tok::r_brace))
     // Ensure that we fall back to the continuation indent width instead of


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60361.194013.patch
Type: text/x-patch
Size: 3282 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190406/f22dc471/attachment-0001.bin>