Skip to content

Commit a77ab97

Browse files
firewavefirewave
authored
Suppressions: some cleanups (cppcheck-opensource#4980)
* Suppressions: merged `isSuppressedLocal()` into `isSuppressed()` * avoid some unnecessary copies when adding suppressions * TestSuppressions: improved readability of multiple line string literals * supressions.h: got rid of unnecessary copy and assignment operators for `Suppressions::Suppression` - fixes `performance-move-const-arg` clang-tidy warning * TestSuppressions: cleaned up a variable construction
1 parent d3bdb84 commit a77ab97

6 files changed

Lines changed: 54 additions & 68 deletions

File tree

‎lib/cppcheck.cpp‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1608,14 +1608,8 @@ void CppCheck::reportErr(const ErrorMessage &msg)
16081608
// TODO: only convert if necessary
16091609
const Suppressions::ErrorMessage errorMessage = msg.toSuppressionsErrorMessage();
16101610

1611-
if (mUseGlobalSuppressions) {
1612-
if (mSettings.nomsg.isSuppressed(errorMessage)) {
1613-
return;
1614-
}
1615-
} else {
1616-
if (mSettings.nomsg.isSuppressedLocal(errorMessage)) {
1617-
return;
1618-
}
1611+
if (mSettings.nomsg.isSuppressed(errorMessage, mUseGlobalSuppressions)) {
1612+
return;
16191613
}
16201614

16211615
if (!mSettings.nofail.isSuppressed(errorMessage) && !mSettings.nomsg.isSuppressed(errorMessage)) {

‎lib/importproject.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1274,8 +1274,7 @@ bool ImportProject::importCppcheckGuiProject(std::istream &istr, Settings *setti
12741274

12751275
for (const std::string &p : paths)
12761276
guiProject.pathNames.push_back(p);
1277-
for (const Suppressions::Suppression &supp : suppressions)
1278-
settings->nomsg.addSuppression(supp);
1277+
settings->nomsg.addSuppressions(std::move(suppressions));
12791278
settings->checkHeaders = temp.checkHeaders;
12801279
settings->checkUnusedTemplates = temp.checkUnusedTemplates;
12811280
settings->maxCtuDepth = temp.maxCtuDepth;

‎lib/preprocessor.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ static void addinlineSuppressions(const simplecpp::TokenList &tokens, const Sett
179179
suppr.fileName = relativeFilename;
180180
suppr.lineNumber = tok->location.line;
181181
suppr.thisAndNextLine = thisAndNextLine;
182-
suppressions.addSuppression(suppr);
182+
suppressions.addSuppression(std::move(suppr));
183183
}
184184
}
185185
}

‎lib/suppressions.cpp‎

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ std::string Suppressions::parseXmlFile(const char *filename)
110110
return "Unknown suppression element \"" + std::string(e2->Name()) + "\", expected id/fileName/lineNumber/symbolName/hash";
111111
}
112112

113-
const std::string err = addSuppression(s);
113+
const std::string err = addSuppression(std::move(s));
114114
if (!err.empty())
115115
return err;
116116
}
@@ -218,10 +218,10 @@ std::string Suppressions::addSuppressionLine(const std::string &line)
218218

219219
suppression.fileName = Path::simplifyPath(suppression.fileName);
220220

221-
return addSuppression(suppression);
221+
return addSuppression(std::move(suppression));
222222
}
223223

224-
std::string Suppressions::addSuppression(const Suppressions::Suppression &suppression)
224+
std::string Suppressions::addSuppression(Suppressions::Suppression suppression)
225225
{
226226
// Check if suppression is already in list
227227
auto foundSuppression = std::find_if(mSuppressions.begin(), mSuppressions.end(),
@@ -253,15 +253,15 @@ std::string Suppressions::addSuppression(const Suppressions::Suppression &suppre
253253
if (!isValidGlobPattern(suppression.fileName))
254254
return "Failed to add suppression. Invalid glob pattern '" + suppression.fileName + "'.";
255255

256-
mSuppressions.push_back(suppression);
256+
mSuppressions.push_back(std::move(suppression));
257257

258258
return "";
259259
}
260260

261-
std::string Suppressions::addSuppressions(const std::list<Suppression> &suppressions)
261+
std::string Suppressions::addSuppressions(std::list<Suppression> suppressions)
262262
{
263-
for (const auto &newSuppression : suppressions) {
264-
auto errmsg = addSuppression(newSuppression);
263+
for (auto &newSuppression : suppressions) {
264+
auto errmsg = addSuppression(std::move(newSuppression));
265265
if (!errmsg.empty())
266266
return errmsg;
267267
}
@@ -367,10 +367,12 @@ std::string Suppressions::Suppression::getText() const
367367
return ret;
368368
}
369369

370-
bool Suppressions::isSuppressed(const Suppressions::ErrorMessage &errmsg)
370+
bool Suppressions::isSuppressed(const Suppressions::ErrorMessage &errmsg, bool global)
371371
{
372372
const bool unmatchedSuppression(errmsg.errorId == "unmatchedSuppression");
373373
for (Suppression &s : mSuppressions) {
374+
if (!global && !s.isLocal())
375+
continue;
374376
if (unmatchedSuppression && s.errorId != errmsg.errorId)
375377
continue;
376378
if (s.isMatch(errmsg))
@@ -386,20 +388,6 @@ bool Suppressions::isSuppressed(const ::ErrorMessage &errmsg)
386388
return isSuppressed(errmsg.toSuppressionsErrorMessage());
387389
}
388390

389-
bool Suppressions::isSuppressedLocal(const Suppressions::ErrorMessage &errmsg)
390-
{
391-
const bool unmatchedSuppression(errmsg.errorId == "unmatchedSuppression");
392-
for (Suppression &s : mSuppressions) {
393-
if (!s.isLocal())
394-
continue;
395-
if (unmatchedSuppression && s.errorId != errmsg.errorId)
396-
continue;
397-
if (s.isMatch(errmsg))
398-
return true;
399-
}
400-
return false;
401-
}
402-
403391
void Suppressions::dump(std::ostream & out) const
404392
{
405393
out << " <suppressions>" << std::endl;

‎lib/suppressions.h‎

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -56,23 +56,8 @@ class CPPCHECKLIB Suppressions {
5656

5757
struct CPPCHECKLIB Suppression {
5858
Suppression() : lineNumber(NO_LINE), hash(0), thisAndNextLine(false), matched(false), checked(false) {}
59-
Suppression(const Suppression &other) {
60-
*this = other;
61-
}
6259
Suppression(std::string id, std::string file, int line=NO_LINE) : errorId(std::move(id)), fileName(std::move(file)), lineNumber(line), hash(0), thisAndNextLine(false), matched(false), checked(false) {}
6360

64-
Suppression & operator=(const Suppression &other) {
65-
errorId = other.errorId;
66-
fileName = other.fileName;
67-
lineNumber = other.lineNumber;
68-
symbolName = other.symbolName;
69-
hash = other.hash;
70-
thisAndNextLine = other.thisAndNextLine;
71-
matched = other.matched;
72-
checked = other.checked;
73-
return *this;
74-
}
75-
7661
bool operator<(const Suppression &other) const {
7762
if (errorId != other.errorId)
7863
return errorId < other.errorId;
@@ -163,21 +148,22 @@ class CPPCHECKLIB Suppressions {
163148
* @param suppression suppression details
164149
* @return error message. empty upon success
165150
*/
166-
std::string addSuppression(const Suppression &suppression);
151+
std::string addSuppression(Suppression suppression);
167152

168153
/**
169154
* @brief Combine list of suppressions into the current suppressions.
170155
* @param suppressions list of suppression details
171156
* @return error message. empty upon success
172157
*/
173-
std::string addSuppressions(const std::list<Suppression> &suppressions);
158+
std::string addSuppressions(std::list<Suppression> suppressions);
174159

175160
/**
176161
* @brief Returns true if this message should not be shown to the user.
177162
* @param errmsg error message
163+
* @param global use global suppressions
178164
* @return true if this error is suppressed.
179165
*/
180-
bool isSuppressed(const ErrorMessage &errmsg);
166+
bool isSuppressed(const ErrorMessage &errmsg, bool global = true);
181167

182168
/**
183169
* @brief Returns true if this message should not be shown to the user.
@@ -186,13 +172,6 @@ class CPPCHECKLIB Suppressions {
186172
*/
187173
bool isSuppressed(const ::ErrorMessage &errmsg);
188174

189-
/**
190-
* @brief Returns true if this message should not be shown to the user, only uses local suppressions.
191-
* @param errmsg error message
192-
* @return true if this error is suppressed.
193-
*/
194-
bool isSuppressedLocal(const ErrorMessage &errmsg);
195-
196175
/**
197176
* @brief Create an xml dump of suppressions
198177
* @param out stream to write XML to

‎test/testsuppressions.cpp‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ class TestSuppressions : public TestFixture {
8282
TEST_CASE(unusedFunction);
8383

8484
TEST_CASE(suppressingSyntaxErrorAndExitCode);
85+
TEST_CASE(suppressLocal);
8586
}
8687

8788
void suppressionsBadId1() const {
@@ -112,15 +113,17 @@ class TestSuppressions : public TestFixture {
112113

113114
void suppressionsDosFormat() const {
114115
Suppressions suppressions;
115-
std::istringstream s("abc\r\ndef\r\n");
116+
std::istringstream s("abc\r\n"
117+
"def\r\n");
116118
ASSERT_EQUALS("", suppressions.parseFile(s));
117119
ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("abc")));
118120
ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("def")));
119121
}
120122

121123
void suppressionsFileNameWithColon() const {
122124
Suppressions suppressions;
123-
std::istringstream s("errorid:c:\\foo.cpp\nerrorid:c:\\bar.cpp:12");
125+
std::istringstream s("errorid:c:\\foo.cpp\n"
126+
"errorid:c:\\bar.cpp:12");
124127
ASSERT_EQUALS("", suppressions.parseFile(s));
125128
ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "c:/foo.cpp", 1111)));
126129
ASSERT_EQUALS(false, suppressions.isSuppressed(errorMessage("errorid", "c:/bar.cpp", 10)));
@@ -138,7 +141,9 @@ class TestSuppressions : public TestFixture {
138141
// Check that globbing works
139142
{
140143
Suppressions suppressions;
141-
std::istringstream s("errorid:x*.cpp\nerrorid:y?.cpp\nerrorid:test.c*");
144+
std::istringstream s("errorid:x*.cpp\n"
145+
"errorid:y?.cpp\n"
146+
"errorid:test.c*");
142147
ASSERT_EQUALS("", suppressions.parseFile(s));
143148
ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "xyz.cpp", 1)));
144149
ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "xyz.cpp.cpp", 1)));
@@ -152,7 +157,10 @@ class TestSuppressions : public TestFixture {
152157
// Check that both a filename match and a glob match apply
153158
{
154159
Suppressions suppressions;
155-
std::istringstream s("errorid:x*.cpp\nerrorid:xyz.cpp:1\nerrorid:a*.cpp:1\nerrorid:abc.cpp:2");
160+
std::istringstream s("errorid:x*.cpp\n"
161+
"errorid:xyz.cpp:1\n"
162+
"errorid:a*.cpp:1\n"
163+
"errorid:abc.cpp:2");
156164
ASSERT_EQUALS("", suppressions.parseFile(s));
157165
ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "xyz.cpp", 1)));
158166
ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "xyz.cpp", 2)));
@@ -526,12 +534,14 @@ class TestSuppressions : public TestFixture {
526534
}
527535

528536
void suppressionsFileComment() const {
529-
std::istringstream file1("# comment\nabc");
537+
std::istringstream file1("# comment\n"
538+
"abc");
530539
Suppressions suppressions1;
531540
suppressions1.parseFile(file1);
532541
ASSERT_EQUALS(true, suppressions1.isSuppressed(errorMessage("abc", "test.cpp", 123)));
533542

534-
std::istringstream file2("// comment\nabc");
543+
std::istringstream file2("// comment\n"
544+
"abc");
535545
Suppressions suppressions2;
536546
suppressions2.parseFile(file2);
537547
ASSERT_EQUALS(true, suppressions2.isSuppressed(errorMessage("abc", "test.cpp", 123)));
@@ -700,9 +710,9 @@ class TestSuppressions : public TestFixture {
700710

701711
void inlinesuppress_unusedFunction() const { // #4210, #4946 - wrong report of "unmatchedSuppression" for "unusedFunction"
702712
Suppressions suppressions;
703-
auto suppression = Suppressions::Suppression("unusedFunction", "test.c", 3);
713+
Suppressions::Suppression suppression("unusedFunction", "test.c", 3);
704714
suppression.checked = true; // have to do this because fixes for #5704
705-
suppressions.addSuppression(suppression);
715+
suppressions.addSuppression(std::move(suppression));
706716
ASSERT_EQUALS(true, !suppressions.getUnmatchedLocalSuppressions("test.c", true).empty());
707717
ASSERT_EQUALS(false, !suppressions.getUnmatchedGlobalSuppressions(true).empty());
708718
ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions("test.c", false).empty());
@@ -795,9 +805,11 @@ class TestSuppressions : public TestFixture {
795805
ASSERT_EQUALS(false, s.isSuppressed(errorMsg));
796806
errorMsg.symbolNames = "array1\n";
797807
ASSERT_EQUALS(true, s.isSuppressed(errorMsg));
798-
errorMsg.symbolNames = "x\narray2\n";
808+
errorMsg.symbolNames = "x\n"
809+
"array2\n";
799810
ASSERT_EQUALS(true, s.isSuppressed(errorMsg));
800-
errorMsg.symbolNames = "array3\nx\n";
811+
errorMsg.symbolNames = "array3\n"
812+
"x\n";
801813
ASSERT_EQUALS(true, s.isSuppressed(errorMsg));
802814
}
803815

@@ -833,6 +845,20 @@ class TestSuppressions : public TestFixture {
833845
ASSERT_EQUALS(2, checkSuppression(code3, "zerodiv:test.cpp:3")); // suppress 'errordiv' at line 3 of test.cpp
834846
}
835847

848+
void suppressLocal() const {
849+
Suppressions suppressions;
850+
std::istringstream s("errorid:test.cpp\n"
851+
"errorid2");
852+
ASSERT_EQUALS("", suppressions.parseFile(s));
853+
ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "test.cpp", 1)));
854+
ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "test.cpp", 1), false));
855+
ASSERT_EQUALS(false, suppressions.isSuppressed(errorMessage("errorid", "test2.cpp", 1)));
856+
ASSERT_EQUALS(false, suppressions.isSuppressed(errorMessage("errorid", "test2.cpp", 1), false));
857+
ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid2", "test.cpp", 1)));
858+
ASSERT_EQUALS(false, suppressions.isSuppressed(errorMessage("errorid2", "test.cpp", 1), false));
859+
ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid2", "test2.cpp", 1)));
860+
ASSERT_EQUALS(false, suppressions.isSuppressed(errorMessage("errorid2", "test2.cpp", 1), false));
861+
}
836862
};
837863

838864
REGISTER_TEST(TestSuppressions)

0 commit comments

Comments
 (0)