release/21.x: [clang-format] Correctly handle backward compatibility of C headers (#159908)#163919
Merged
c-rhodes merged 2 commits intollvm:release/21.xfrom Oct 20, 2025
Merged
release/21.x: [clang-format] Correctly handle backward compatibility of C headers (#159908)#163919c-rhodes merged 2 commits intollvm:release/21.xfrom
c-rhodes merged 2 commits intollvm:release/21.xfrom
Conversation
Member
Author
|
@mydeveloperday @HazardyKnusperkeks What do you think about merging this PR to the release branch? |
Member
Author
|
@llvm/pr-subscribers-clang-format Author: None (llvmbot) ChangesRequested by: @owenca Full diff: https://github.com/llvm/llvm-project/pull/163919.diff 2 Files Affected:
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 161a6c4b47e7f..5bdb810a3925b 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -2132,48 +2132,70 @@ std::error_code parseConfiguration(llvm::MemoryBufferRef Config,
Input >> Styles;
if (Input.error())
return Input.error();
+ if (Styles.empty())
+ return make_error_code(ParseError::Success);
+
+ const auto StyleCount = Styles.size();
- for (unsigned i = 0; i < Styles.size(); ++i) {
- // Ensures that only the first configuration can skip the Language option.
- if (Styles[i].Language == FormatStyle::LK_None && i != 0)
+ // Start from the second style as (only) the first one may be the default.
+ for (unsigned I = 1; I < StyleCount; ++I) {
+ const auto Lang = Styles[I].Language;
+ if (Lang == FormatStyle::LK_None)
return make_error_code(ParseError::Error);
// Ensure that each language is configured at most once.
- for (unsigned j = 0; j < i; ++j) {
- if (Styles[i].Language == Styles[j].Language) {
+ for (unsigned J = 0; J < I; ++J) {
+ if (Lang == Styles[J].Language) {
LLVM_DEBUG(llvm::dbgs()
<< "Duplicate languages in the config file on positions "
- << j << " and " << i << "\n");
+ << J << " and " << I << '\n');
return make_error_code(ParseError::Error);
}
}
}
- // Look for a suitable configuration starting from the end, so we can
- // find the configuration for the specific language first, and the default
- // configuration (which can only be at slot 0) after it.
- FormatStyle::FormatStyleSet StyleSet;
- bool LanguageFound = false;
- for (const FormatStyle &Style : llvm::reverse(Styles)) {
- const auto Lang = Style.Language;
- if (Lang != FormatStyle::LK_None)
- StyleSet.Add(Style);
- if (Lang == Language ||
- // For backward compatibility.
- (Lang == FormatStyle::LK_Cpp && Language == FormatStyle::LK_C)) {
- LanguageFound = true;
- } else if (IsDotHFile && Language == FormatStyle::LK_Cpp &&
- (Lang == FormatStyle::LK_C || Lang == FormatStyle::LK_ObjC)) {
- Language = Lang;
- LanguageFound = true;
+
+ int LanguagePos = -1; // Position of the style for Language.
+ int CppPos = -1; // Position of the style for C++.
+ int CPos = -1; // Position of the style for C.
+
+ // Search Styles for Language and store the positions of C++ and C styles in
+ // case Language is not found.
+ for (unsigned I = 0; I < StyleCount; ++I) {
+ const auto Lang = Styles[I].Language;
+ if (Lang == Language) {
+ LanguagePos = I;
+ break;
}
- }
- if (!LanguageFound) {
- if (Styles.empty() || Styles[0].Language != FormatStyle::LK_None)
+ if (Lang == FormatStyle::LK_Cpp)
+ CppPos = I;
+ else if (Lang == FormatStyle::LK_C)
+ CPos = I;
+ }
+
+ // If Language is not found, use the default style if there is one. Otherwise,
+ // use the C style for C++ .h files and for backward compatibility, the C++
+ // style for .c files.
+ if (LanguagePos < 0) {
+ if (Styles[0].Language == FormatStyle::LK_None) // Default style.
+ LanguagePos = 0;
+ else if (IsDotHFile && Language == FormatStyle::LK_Cpp)
+ LanguagePos = CPos;
+ else if (!IsDotHFile && Language == FormatStyle::LK_C)
+ LanguagePos = CppPos;
+ if (LanguagePos < 0)
return make_error_code(ParseError::Unsuitable);
- FormatStyle DefaultStyle = Styles[0];
- DefaultStyle.Language = Language;
- StyleSet.Add(std::move(DefaultStyle));
}
- *Style = *StyleSet.Get(Language);
+
+ for (const auto &S : llvm::reverse(llvm::drop_begin(Styles)))
+ Style->StyleSet.Add(S);
+
+ *Style = Styles[LanguagePos];
+
+ if (LanguagePos == 0) {
+ if (Style->Language == FormatStyle::LK_None) // Default style.
+ Style->Language = Language;
+ Style->StyleSet.Add(*Style);
+ }
+
if (Style->InsertTrailingCommas != FormatStyle::TCS_None &&
Style->BinPackArguments) {
// See comment on FormatStyle::TSC_Wrapped.
@@ -2204,14 +2226,8 @@ FormatStyle::FormatStyleSet::Get(FormatStyle::LanguageKind Language) const {
if (!Styles)
return std::nullopt;
auto It = Styles->find(Language);
- if (It == Styles->end()) {
- if (Language != FormatStyle::LK_C)
- return std::nullopt;
- // For backward compatibility.
- It = Styles->find(FormatStyle::LK_Cpp);
- if (It == Styles->end())
- return std::nullopt;
- }
+ if (It == Styles->end())
+ return std::nullopt;
FormatStyle Style = It->second;
Style.StyleSet = *this;
return Style;
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 2b17c36f6aa84..ff42f09b90cf3 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -1249,6 +1249,13 @@ TEST(ConfigParseTest, ParsesConfigurationWithLanguages) {
IndentWidth, 56u);
}
+TEST(ConfigParseTest, AllowCommentOnlyConfigFile) {
+ FormatStyle Style = {};
+ Style.Language = FormatStyle::LK_Cpp;
+ EXPECT_EQ(parseConfiguration("#Language: C", &Style), ParseError::Success);
+ EXPECT_EQ(Style.Language, FormatStyle::LK_Cpp);
+}
+
TEST(ConfigParseTest, AllowCppForC) {
FormatStyle Style = {};
Style.Language = FormatStyle::LK_C;
@@ -1269,7 +1276,7 @@ TEST(ConfigParseTest, AllowCppForC) {
ParseError::Success);
}
-TEST(ConfigParseTest, HandleNonCppDotHFile) {
+TEST(ConfigParseTest, HandleDotHFile) {
FormatStyle Style = {};
Style.Language = FormatStyle::LK_Cpp;
EXPECT_EQ(parseConfiguration("Language: C", &Style,
@@ -1280,11 +1287,14 @@ TEST(ConfigParseTest, HandleNonCppDotHFile) {
Style = {};
Style.Language = FormatStyle::LK_Cpp;
- EXPECT_EQ(parseConfiguration("Language: ObjC", &Style,
+ EXPECT_EQ(parseConfiguration("Language: Cpp\n"
+ "...\n"
+ "Language: C",
+ &Style,
/*AllowUnknownOptions=*/false,
/*IsDotHFile=*/true),
ParseError::Success);
- EXPECT_EQ(Style.Language, FormatStyle::LK_ObjC);
+ EXPECT_EQ(Style.Language, FormatStyle::LK_Cpp);
}
TEST(ConfigParseTest, UsesLanguageForBasedOnStyle) {
|
HazardyKnusperkeks
approved these changes
Oct 17, 2025
…lvm#159908) This in effect reverts 05fb840 and 7e1a88b, the latter of which erroneously changed the behavior of formatting `ObjC` header files when both the default and `ObjC` styles were absent. Now the previous behavior of treating that as an error is restored. Fixes llvm#158704 (cherry picked from commit d7921de)
…lvm#163111) (cherry picked from commit 059f2df)
|
@owenca (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Also allow comment-only config files (#163111).
Backport d7921de 059f2df
Requested by: @owenca