Skip to content

Commit 29fa5b7

Browse files
authored
Revert "[Clang] improve -Wstring-concatenation to warn on every missing comma in initializer lists" (#154369)
Revert #154018 changes due to excessive _false positives_. The warning caused multiple benign reports in large codebases (e.g. _Linux kernel_, _Fuchsia_, _tcpdump_). Since many of these concatenations are intentional and follow project style rules, the diagnostic introduced more false positives than value. This will be revisited as a potential `clang-tidy` check instead.
1 parent d2b2d6f commit 29fa5b7

File tree

3 files changed

+19
-54
lines changed

3 files changed

+19
-54
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,6 @@ Improvements to Clang's diagnostics
168168
an override of a virtual method.
169169
- Fixed fix-it hint for fold expressions. Clang now correctly places the suggested right
170170
parenthesis when diagnosing malformed fold expressions. (#GH151787)
171-
- ``-Wstring-concatenation`` now diagnoses every missing comma in an initializer list,
172-
rather than stopping after the first. (#GH153745)
173171

174172
- Fixed an issue where emitted format-signedness diagnostics were not associated with an appropriate
175173
diagnostic id. Besides being incorrect from an API standpoint, this was user visible, e.g.:

clang/lib/Sema/SemaDecl.cpp

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14708,14 +14708,7 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
1470814708
isa<InitListExpr>(var->getInit())) {
1470914709
const auto *ILE = cast<InitListExpr>(var->getInit());
1471014710
unsigned NumInits = ILE->getNumInits();
14711-
if (NumInits > 2) {
14712-
auto concatenatedPartsAt = [&](unsigned Index) -> unsigned {
14713-
if (const Expr *E = ILE->getInit(Index))
14714-
if (const auto *S = dyn_cast<StringLiteral>(E->IgnoreImpCasts()))
14715-
return S->getNumConcatenated();
14716-
return 0;
14717-
};
14718-
14711+
if (NumInits > 2)
1471914712
for (unsigned I = 0; I < NumInits; ++I) {
1472014713
const auto *Init = ILE->getInit(I);
1472114714
if (!Init)
@@ -14728,33 +14721,35 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
1472814721
// Diagnose missing comma in string array initialization.
1472914722
// Do not warn when all the elements in the initializer are concatenated
1473014723
// together. Do not warn for macros too.
14731-
if (NumConcat == 2) {
14732-
if (SL->getBeginLoc().isMacroID())
14733-
continue;
14734-
14735-
unsigned L = I > 0 ? concatenatedPartsAt(I - 1) : 0;
14736-
unsigned R = I + 1 < NumInits ? concatenatedPartsAt(I + 1) : 0;
14737-
14738-
// Skip neighbors with multi-part concatenations.
14739-
if (R > 1)
14740-
continue;
14724+
if (NumConcat == 2 && !SL->getBeginLoc().isMacroID()) {
14725+
bool OnlyOneMissingComma = true;
14726+
for (unsigned J = I + 1; J < NumInits; ++J) {
14727+
const auto *Init = ILE->getInit(J);
14728+
if (!Init)
14729+
break;
14730+
const auto *SLJ = dyn_cast<StringLiteral>(Init->IgnoreImpCasts());
14731+
if (!SLJ || SLJ->getNumConcatenated() > 1) {
14732+
OnlyOneMissingComma = false;
14733+
break;
14734+
}
14735+
}
1474114736

14742-
// Diagnose when at least one neighbor is a single literal.
14743-
if (R == 1 || L == 1) {
14737+
if (OnlyOneMissingComma) {
1474414738
SmallVector<FixItHint, 1> Hints;
14745-
// Insert a comma between the two tokens of this element.
14746-
Hints.push_back(FixItHint::CreateInsertion(
14747-
PP.getLocForEndOfToken(SL->getStrTokenLoc(0)), ", "));
14739+
for (unsigned i = 0; i < NumConcat - 1; ++i)
14740+
Hints.push_back(FixItHint::CreateInsertion(
14741+
PP.getLocForEndOfToken(SL->getStrTokenLoc(i)), ","));
1474814742

1474914743
Diag(SL->getStrTokenLoc(1),
1475014744
diag::warn_concatenated_literal_array_init)
1475114745
<< Hints;
1475214746
Diag(SL->getBeginLoc(),
1475314747
diag::note_concatenated_string_literal_silence);
1475414748
}
14749+
// In any case, stop now.
14750+
break;
1475514751
}
1475614752
}
14757-
}
1475814753
}
1475914754

1476014755

clang/test/Sema/string-concat.c

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -168,31 +168,3 @@ const char *extra_parens_to_suppress_warning[] = {
168168
"promise"),
169169
"shared_future"
170170
};
171-
172-
const char *multiple_missing_commas1[] = {
173-
"1",
174-
"2" // expected-note {{place parentheses around the string literal to silence warning}}
175-
"3", // expected-warning {{suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma?}}
176-
"4",
177-
"5",
178-
"6" // expected-note {{place parentheses around the string literal to silence warning}}
179-
"7", // expected-warning {{suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma?}}
180-
"8",
181-
"9",
182-
"10",
183-
"11",
184-
};
185-
186-
const char *multiple_missing_commas2[] = {
187-
"1",
188-
"2"
189-
"3"
190-
"4"
191-
"5",
192-
"6" // expected-note {{place parentheses around the string literal to silence warning}}
193-
"7", // expected-warning {{suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma?}}
194-
"8",
195-
"9",
196-
"10",
197-
"11",
198-
};

0 commit comments

Comments
 (0)