Author: Zhao Wei Liew Date: 2022-01-03T11:36:00+01:00 New Revision: 0090cd4e7a24bedeb24dfe5b3b55167ad74e231e
URL: https://github.com/llvm/llvm-project/commit/0090cd4e7a24bedeb24dfe5b3b55167ad74e231e DIFF: https://github.com/llvm/llvm-project/commit/0090cd4e7a24bedeb24dfe5b3b55167ad74e231e.diff LOG: [clang-format] Support inheriting from more than 1 parents in the fallback case 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. Reviewed By: HazardyKnusperkeks, curdeius Differential Revision: https://reviews.llvm.org/D116371 Added: Modified: clang/lib/Format/Format.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index be01daa38929d..f3c337a928228 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -3288,6 +3288,16 @@ llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, 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)) { @@ -3330,14 +3340,7 @@ llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, 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; } @@ -3363,17 +3366,9 @@ llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, 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; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 005fc4db9159b..bb344d4383ea6 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -21464,8 +21464,8 @@ TEST(FormatStyle, GetStyleOfFile) { 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" @@ -21480,6 +21480,25 @@ TEST(FormatStyle, GetStyleOfFile) { 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)); @@ -21503,15 +21522,7 @@ TEST(FormatStyle, GetStyleOfFile) { 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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
