Skip to content

Commit d7921de

Browse files
authored
[clang-format] Correctly handle backward compatibility of C headers (#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 #158704
1 parent 5f44fa8 commit d7921de

File tree

2 files changed

+59
-41
lines changed

2 files changed

+59
-41
lines changed

clang/lib/Format/Format.cpp

Lines changed: 53 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2185,47 +2185,68 @@ std::error_code parseConfiguration(llvm::MemoryBufferRef Config,
21852185
if (Input.error())
21862186
return Input.error();
21872187

2188-
for (unsigned i = 0; i < Styles.size(); ++i) {
2189-
// Ensures that only the first configuration can skip the Language option.
2190-
if (Styles[i].Language == FormatStyle::LK_None && i != 0)
2188+
assert(!Styles.empty());
2189+
const auto StyleCount = Styles.size();
2190+
2191+
// Start from the second style as (only) the first one may be the default.
2192+
for (unsigned I = 1; I < StyleCount; ++I) {
2193+
const auto Lang = Styles[I].Language;
2194+
if (Lang == FormatStyle::LK_None)
21912195
return make_error_code(ParseError::Error);
21922196
// Ensure that each language is configured at most once.
2193-
for (unsigned j = 0; j < i; ++j) {
2194-
if (Styles[i].Language == Styles[j].Language) {
2197+
for (unsigned J = 0; J < I; ++J) {
2198+
if (Lang == Styles[J].Language) {
21952199
LLVM_DEBUG(llvm::dbgs()
21962200
<< "Duplicate languages in the config file on positions "
2197-
<< j << " and " << i << "\n");
2201+
<< J << " and " << I << '\n');
21982202
return make_error_code(ParseError::Error);
21992203
}
22002204
}
22012205
}
2202-
// Look for a suitable configuration starting from the end, so we can
2203-
// find the configuration for the specific language first, and the default
2204-
// configuration (which can only be at slot 0) after it.
2205-
FormatStyle::FormatStyleSet StyleSet;
2206-
bool LanguageFound = false;
2207-
for (const FormatStyle &Style : llvm::reverse(Styles)) {
2208-
const auto Lang = Style.Language;
2209-
if (Lang != FormatStyle::LK_None)
2210-
StyleSet.Add(Style);
2211-
if (Lang == Language ||
2212-
// For backward compatibility.
2213-
(Lang == FormatStyle::LK_Cpp && Language == FormatStyle::LK_C)) {
2214-
LanguageFound = true;
2215-
} else if (IsDotHFile && Language == FormatStyle::LK_Cpp &&
2216-
(Lang == FormatStyle::LK_C || Lang == FormatStyle::LK_ObjC)) {
2217-
Language = Lang;
2218-
LanguageFound = true;
2206+
2207+
int LanguagePos = -1; // Position of the style for Language.
2208+
int CppPos = -1; // Position of the style for C++.
2209+
int CPos = -1; // Position of the style for C.
2210+
2211+
// Search Styles for Language and store the positions of C++ and C styles in
2212+
// case Language is not found.
2213+
for (unsigned I = 0; I < StyleCount; ++I) {
2214+
const auto Lang = Styles[I].Language;
2215+
if (Lang == Language) {
2216+
LanguagePos = I;
2217+
break;
22192218
}
2220-
}
2221-
if (!LanguageFound) {
2222-
if (Styles.empty() || Styles[0].Language != FormatStyle::LK_None)
2219+
if (Lang == FormatStyle::LK_Cpp)
2220+
CppPos = I;
2221+
else if (Lang == FormatStyle::LK_C)
2222+
CPos = I;
2223+
}
2224+
2225+
// If Language is not found, use the default style if there is one. Otherwise,
2226+
// use the C style for C++ .h files and for backward compatibility, the C++
2227+
// style for .c files.
2228+
if (LanguagePos < 0) {
2229+
if (Styles[0].Language == FormatStyle::LK_None) // Default style.
2230+
LanguagePos = 0;
2231+
else if (IsDotHFile && Language == FormatStyle::LK_Cpp)
2232+
LanguagePos = CPos;
2233+
else if (!IsDotHFile && Language == FormatStyle::LK_C)
2234+
LanguagePos = CppPos;
2235+
if (LanguagePos < 0)
22232236
return make_error_code(ParseError::Unsuitable);
2224-
FormatStyle DefaultStyle = Styles[0];
2225-
DefaultStyle.Language = Language;
2226-
StyleSet.Add(std::move(DefaultStyle));
22272237
}
2228-
*Style = *StyleSet.Get(Language);
2238+
2239+
for (const auto &S : llvm::reverse(llvm::drop_begin(Styles)))
2240+
Style->StyleSet.Add(S);
2241+
2242+
*Style = Styles[LanguagePos];
2243+
2244+
if (LanguagePos == 0) {
2245+
if (Style->Language == FormatStyle::LK_None) // Default style.
2246+
Style->Language = Language;
2247+
Style->StyleSet.Add(*Style);
2248+
}
2249+
22292250
if (Style->InsertTrailingCommas != FormatStyle::TCS_None &&
22302251
Style->BinPackArguments) {
22312252
// See comment on FormatStyle::TSC_Wrapped.
@@ -2256,14 +2277,8 @@ FormatStyle::FormatStyleSet::Get(FormatStyle::LanguageKind Language) const {
22562277
if (!Styles)
22572278
return std::nullopt;
22582279
auto It = Styles->find(Language);
2259-
if (It == Styles->end()) {
2260-
if (Language != FormatStyle::LK_C)
2261-
return std::nullopt;
2262-
// For backward compatibility.
2263-
It = Styles->find(FormatStyle::LK_Cpp);
2264-
if (It == Styles->end())
2265-
return std::nullopt;
2266-
}
2280+
if (It == Styles->end())
2281+
return std::nullopt;
22672282
FormatStyle Style = It->second;
22682283
Style.StyleSet = *this;
22692284
return Style;

clang/unittests/Format/ConfigParseTest.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1284,7 +1284,7 @@ TEST(ConfigParseTest, AllowCppForC) {
12841284
ParseError::Success);
12851285
}
12861286

1287-
TEST(ConfigParseTest, HandleNonCppDotHFile) {
1287+
TEST(ConfigParseTest, HandleDotHFile) {
12881288
FormatStyle Style = {};
12891289
Style.Language = FormatStyle::LK_Cpp;
12901290
EXPECT_EQ(parseConfiguration("Language: C", &Style,
@@ -1295,11 +1295,14 @@ TEST(ConfigParseTest, HandleNonCppDotHFile) {
12951295

12961296
Style = {};
12971297
Style.Language = FormatStyle::LK_Cpp;
1298-
EXPECT_EQ(parseConfiguration("Language: ObjC", &Style,
1298+
EXPECT_EQ(parseConfiguration("Language: Cpp\n"
1299+
"...\n"
1300+
"Language: C",
1301+
&Style,
12991302
/*AllowUnknownOptions=*/false,
13001303
/*IsDotHFile=*/true),
13011304
ParseError::Success);
1302-
EXPECT_EQ(Style.Language, FormatStyle::LK_ObjC);
1305+
EXPECT_EQ(Style.Language, FormatStyle::LK_Cpp);
13031306
}
13041307

13051308
TEST(ConfigParseTest, UsesLanguageForBasedOnStyle) {

0 commit comments

Comments
 (0)