Skip to content

Commit 2a0c507

Browse files
committed
Preserve duplicate warnings for matching string prefix literals
1 parent 475789b commit 2a0c507

3 files changed

Lines changed: 24 additions & 4 deletions

File tree

lib/astutils.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1608,10 +1608,20 @@ bool isSameExpression(bool macro, const Token *tok1, const Token *tok2, const Se
16081608
if (tok1 == nullptr || tok2 == nullptr)
16091609
return false;
16101610
// An unknown string-prefix macro leaves the literal outside the AST.
1611-
// Comparing only the macro name would ignore the rest of the expression.
1612-
if ((!tok1->isKeyword() && Token::Match(tok1, "%name% %str%")) ||
1613-
(!tok2->isKeyword() && Token::Match(tok2, "%name% %str%")))
1614-
return false;
1611+
// Compare the literal text as well so identical literals still compare equal.
1612+
const bool stringPrefix1 = !tok1->isKeyword() && Token::Match(tok1, "%name% %str%");
1613+
const bool stringPrefix2 = !tok2->isKeyword() && Token::Match(tok2, "%name% %str%");
1614+
if (stringPrefix1 || stringPrefix2) {
1615+
if (!stringPrefix1 || !stringPrefix2)
1616+
return false;
1617+
for (const Token* str1 = tok1->next(), *str2 = tok2->next();
1618+
Token::Match(str1, "%str%") || Token::Match(str2, "%str%");
1619+
str1 = str1->next(), str2 = str2->next()) {
1620+
if (!Token::Match(str1, "%str%") || !Token::Match(str2, "%str%") ||
1621+
str1->str() != str2->str() || !compareTokenFlags(str1, str2, macro))
1622+
return false;
1623+
}
1624+
}
16151625
// tokens needs to be from the same TokenList so no need check standard on both of them
16161626
if (tok1->isCpp()) {
16171627
if (tok1->str() == "." && tok1->astOperand1() && tok1->astOperand1()->str() == "this")

test/testastutils.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,11 @@ class TestAstUtils : public TestFixture {
202202
ASSERT_EQUALS(false, isSameExpression("x == PREFIX \"/a\" || x == PREFIX \"/b\";\n", "==", "==", cpp));
203203
ASSERT_EQUALS(false, isSameExpression("x == PREFIX \"/a\" || x == PREFIX;\n", "==", "==", cpp));
204204
ASSERT_EQUALS(false, isSameExpression("x == PREFIX || x == PREFIX \"/a\";\n", "==", "==", cpp));
205+
ASSERT_EQUALS(true, isSameExpression("x == PREFIX \"/a\" || x == PREFIX \"/a\";\n", "==", "==", cpp));
206+
ASSERT_EQUALS(true, isSameExpression("x == PREFIX \"/a\" \"/b\" || x == PREFIX \"/a\" \"/b\";\n", "==", "==", cpp));
207+
ASSERT_EQUALS(false, isSameExpression("x == PREFIX \"/a\" \"/b\" || x == PREFIX \"/a\" \"/c\";\n", "==", "==", cpp));
208+
ASSERT_EQUALS(false, isSameExpression("x == FIRST \"/a\" || x == SECOND \"/a\";\n", "==", "==", cpp));
209+
ASSERT_EQUALS(false, isSameExpression("x == PREFIX L\"/a\" || x == PREFIX \"/a\";\n", "==", "==", cpp));
205210

206211
// the remaining test cases are not valid C code
207212
if (!cpp)

test/testother.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12170,6 +12170,11 @@ class TestOther : public TestFixture {
1217012170
" : (name.startswith(SRCDIR \"/com/\") || name.startswith(SRCDIR \"/uno/\"));\n"
1217112171
"}\n", dinit(CheckOptions, $.inconclusive = false));
1217212172
ASSERT_EQUALS("", errout_str());
12173+
12174+
check("bool f(StringRef name) {\n"
12175+
" return name == SRCDIR \"/a\" || name == SRCDIR \"/a\";\n"
12176+
"}\n", dinit(CheckOptions, $.inconclusive = false));
12177+
ASSERT_EQUALS("[test.cpp:2:32]: (style) Same expression on both sides of '||'. [duplicateExpression]\n", errout_str());
1217312178
}
1217412179

1217512180
void raceAfterInterlockedDecrement() {

0 commit comments

Comments
 (0)