Skip to content

Commit c2eb895

Browse files
authored
[Clang] improve -Wstring-concatenation to warn on every missing comma in initializer lists (#154018)
Fixes #153745 --- This PR addresses a limitation in `-Wstring-concatenation`, where only the first missing comma in an initializer list was diagnosed.
1 parent 8a0b3cc commit c2eb895

File tree

3 files changed

+54
-19
lines changed

3 files changed

+54
-19
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@ 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)
171173

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

clang/lib/Sema/SemaDecl.cpp

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14708,7 +14708,14 @@ 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)
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+
1471214719
for (unsigned I = 0; I < NumInits; ++I) {
1471314720
const auto *Init = ILE->getInit(I);
1471414721
if (!Init)
@@ -14721,35 +14728,33 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
1472114728
// Diagnose missing comma in string array initialization.
1472214729
// Do not warn when all the elements in the initializer are concatenated
1472314730
// together. Do not warn for macros too.
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-
}
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;
1473614741

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

1474314749
Diag(SL->getStrTokenLoc(1),
1474414750
diag::warn_concatenated_literal_array_init)
1474514751
<< Hints;
1474614752
Diag(SL->getBeginLoc(),
1474714753
diag::note_concatenated_string_literal_silence);
1474814754
}
14749-
// In any case, stop now.
14750-
break;
1475114755
}
1475214756
}
14757+
}
1475314758
}
1475414759

1475514760

clang/test/Sema/string-concat.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,3 +168,31 @@ 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)