[clang-format] Support inheriting from more than 1 parents in the fallback case

[zwliew]

Summary

Currently, we are unable to inherit from a chain of parent configs where the outermost parent config has BasedOnStyle: InheritParentConfig set. This patch adds a test case for this scenario, and adds support for it.

To illustrate, suppose we have the following directory structure:

- e/
  |- .clang-format (BasedOnStyle: InheritParentConfig) <-- outermost config
  |- sub/
    |- .clang-format (BasedOnStyle: InheritParentConfig)
    |- sub/
      |- .clang-format (BasedOnStyle: InheritParentConfig)
      |- code.cpp

Now consider what happens when we run clang-format --style=file /e/sub/sub/code.cpp.

Without this patch, on a release build, only the innermost config will be applied. On a debug build, clang-format crashes due to an assertion failure.
With this patch, clang-format behaves as we'd expect, applying all 3 configs.

[HazardyKnusperkeks]

Thanks for fixing that.

[curdeius]

LGTM. Thanks for splitting this.

[zwliew]

Rebased on master for context.

clang/lib/Format/Format.cpp

if (std::error_code EC = FS->makeAbsolute(Path))
     return make_string_error(EC.message());
 
   llvm::SmallVector<std::string, 2> FilesToLookFor;
   FilesToLookFor.push_back(".clang-format");
   FilesToLookFor.push_back("_clang-format");
 
   auto dropDiagnosticHandler = [](const llvm::SMDiagnostic &, void *) {};
 
+  auto applyChildFormatTexts = [&](FormatStyle *Style) {
+    for (const auto &MemBuf : llvm::reverse(ChildFormatTextToApply)) {
+      auto EC = parseConfiguration(*MemBuf, Style, AllowUnknownOptions,
+                                   dropDiagnosticHandler);
+      // It was already correctly parsed.
+      assert(!EC);
+      static_cast<void>(EC);
+    }
+  };
+
   for (StringRef Directory = Path; !Directory.empty();
        Directory = llvm::sys::path::parent_path(Directory)) {
 
     auto Status = FS->status(Directory);
     if (!Status ||
         Status->getType() != llvm::sys::fs::file_type::directory_file) {
       continue;
     }
for (const auto &F : FilesToLookFor) {
         LLVM_DEBUG(llvm::dbgs()
                    << "Using configuration file " << ConfigFile << "\n");
 
         if (!Style.InheritsParentConfig) {
           if (ChildFormatTextToApply.empty())
             return Style;
 
           LLVM_DEBUG(llvm::dbgs() << "Applying child configurations\n");
-
-          for (const auto &MemBuf : llvm::reverse(ChildFormatTextToApply)) {
-            auto Ec = parseConfiguration(*MemBuf, &Style, AllowUnknownOptions,
-                                         dropDiagnosticHandler);
-            // It was already correctly parsed.
-            assert(!Ec);
-            static_cast<void>(Ec);
-          }
+          applyChildFormatTexts(&Style);
 
           return Style;
         }
 
         LLVM_DEBUG(llvm::dbgs() << "Inherits parent configuration\n");
 
         // Reset inheritance of style
         Style.InheritsParentConfig = false;
for (StringRef Directory = Path; !Directory.empty();
     }
   }
   if (!UnsuitableConfigFiles.empty())
     return make_string_error("Configuration file(s) do(es) not support " +
                              getLanguageName(Style.Language) + ": " +
                              UnsuitableConfigFiles);
 
   if (!ChildFormatTextToApply.empty()) {
-    assert(ChildFormatTextToApply.size() == 1);
-
     LLVM_DEBUG(llvm::dbgs()
-               << "Applying child configuration on fallback style\n");
-
-    auto Ec =
-        parseConfiguration(*ChildFormatTextToApply.front(), &FallbackStyle,
-                           AllowUnknownOptions, dropDiagnosticHandler);
-    // It was already correctly parsed.
-    assert(!Ec);
-    static_cast<void>(Ec);
+               << "Applying child configurations on fallback style\n");
+    applyChildFormatTexts(&FallbackStyle);
   }
 
   return FallbackStyle;
 }
 
 } // namespace format
 } // namespace clang

clang/unittests/Format/FormatTest.cpp

TEST(FormatStyle, GetStyleOfFile) {
   auto Style7b = getStyle("file", "/d/.clang-format", "LLVM", "", &FS, true);
   ASSERT_TRUE((bool)Style7b);
 
   // Test 8: inferred per-language defaults apply.
   auto StyleTd = getStyle("file", "x.td", "llvm", "", &FS);
   ASSERT_TRUE((bool)StyleTd);
   ASSERT_EQ(*StyleTd, getLLVMStyle(FormatStyle::LK_TableGen));
 
-  // Test 9.1: overwriting a file style, when parent no file exists with no
-  // fallback style
+  // Test 9.1.1: overwriting a file style, when no parent file exists with no
+  // fallback style.
   ASSERT_TRUE(FS.addFile(
       "/e/sub/.clang-format", 0,
       llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: InheritParentConfig\n"
                                        "ColumnLimit: 20")));
   ASSERT_TRUE(FS.addFile("/e/sub/code.cpp", 0,
                          llvm::MemoryBuffer::getMemBuffer("int i;")));
   auto Style9 = getStyle("file", "/e/sub/code.cpp", "none", "", &FS);
   ASSERT_TRUE(static_cast<bool>(Style9));
   ASSERT_EQ(*Style9, [] {
     auto Style = getNoStyle();
     Style.ColumnLimit = 20;
     return Style;
   }());
 
+  // Test 9.1.2: propagate more than one level with no parent file.
+  ASSERT_TRUE(FS.addFile("/e/sub/sub/code.cpp", 0,
+                         llvm::MemoryBuffer::getMemBuffer("int i;")));
+  ASSERT_TRUE(FS.addFile("/e/sub/sub/.clang-format", 0,
+                         llvm::MemoryBuffer::getMemBuffer(
+                             "BasedOnStyle: InheritParentConfig\n"
+                             "WhitespaceSensitiveMacros: ['FOO', 'BAR']")));
+  std::vector<std::string> NonDefaultWhiteSpaceMacros{"FOO", "BAR"};
+
+  ASSERT_NE(Style9->WhitespaceSensitiveMacros, NonDefaultWhiteSpaceMacros);
+  Style9 = getStyle("file", "/e/sub/sub/code.cpp", "none", "", &FS);
+  ASSERT_TRUE(static_cast<bool>(Style9));
+  ASSERT_EQ(*Style9, [&NonDefaultWhiteSpaceMacros] {
+    auto Style = getNoStyle();
+    Style.ColumnLimit = 20;
+    Style.WhitespaceSensitiveMacros = NonDefaultWhiteSpaceMacros;
+    return Style;
+  }());
+
   // Test 9.2: with LLVM fallback style
   Style9 = getStyle("file", "/e/sub/code.cpp", "LLVM", "", &FS);
   ASSERT_TRUE(static_cast<bool>(Style9));
   ASSERT_EQ(*Style9, [] {
     auto Style = getLLVMStyle();
     Style.ColumnLimit = 20;
     return Style;
   }());
 
   // Test 9.3: with a parent file
   ASSERT_TRUE(
       FS.addFile("/e/.clang-format", 0,
                  llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google\n"
                                                   "UseTab: Always")));
   Style9 = getStyle("file", "/e/sub/code.cpp", "none", "", &FS);
   ASSERT_TRUE(static_cast<bool>(Style9));
   ASSERT_EQ(*Style9, [] {
     auto Style = getGoogleStyle();
     Style.ColumnLimit = 20;
     Style.UseTab = FormatStyle::UT_Always;
     return Style;
   }());
 
-  // Test 9.4: propagate more than one level
-  ASSERT_TRUE(FS.addFile("/e/sub/sub/code.cpp", 0,
-                         llvm::MemoryBuffer::getMemBuffer("int i;")));
-  ASSERT_TRUE(FS.addFile("/e/sub/sub/.clang-format", 0,
-                         llvm::MemoryBuffer::getMemBuffer(
-                             "BasedOnStyle: InheritParentConfig\n"
-                             "WhitespaceSensitiveMacros: ['FOO', 'BAR']")));
-  std::vector<std::string> NonDefaultWhiteSpaceMacros{"FOO", "BAR"};
-
+  // Test 9.4: propagate more than one level with a parent file.
   const auto SubSubStyle = [&NonDefaultWhiteSpaceMacros] {
     auto Style = getGoogleStyle();
     Style.ColumnLimit = 20;
     Style.UseTab = FormatStyle::UT_Always;
     Style.WhitespaceSensitiveMacros = NonDefaultWhiteSpaceMacros;
     return Style;
   }();