Skip to content

Commit

Permalink
General code improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
JohanBertrand committed Sep 4, 2023
1 parent 19b3cef commit 1ed3d98
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 40 deletions.
71 changes: 35 additions & 36 deletions lib/preprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ Preprocessor::~Preprocessor()

namespace {
struct BadInlineSuppression {
BadInlineSuppression(const std::string file, const int line, std::string msg) : file(file), line(line), errmsg(std::move(msg)) {}
BadInlineSuppression(const std::string &file, const int line, std::string msg) : file(file), line(line), errmsg(std::move(msg)) {}
std::string file;
int line;
std::string errmsg;
Expand All @@ -99,23 +99,23 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:

// check if it has a prefix
const std::string::size_type posEndComment = comment.find_first_of(" [", pos1+cppchecksuppress.size());

// skip spaces after "cppcheck-suppress" and its possible prefix
const std::string::size_type pos2 = comment.find_first_not_of(' ', posEndComment);
if (pos2 == std::string::npos)
return false;

Suppressions::Type errorType = Suppressions::Type::unique;

// determine prefix if specified
if (posEndComment >= (pos1 + cppchecksuppress.size() + 1)) {
// determine prefix if specified
if (posEndComment >= (pos1 + cppchecksuppress.size() + 1)) {
if (comment.at(pos1 + cppchecksuppress.size()) != '-')
return false;

const unsigned int argumentLength =
const unsigned int argumentLength =
posEndComment - (pos1 + cppchecksuppress.size() + 1);

const std::string suppressTypeString =
const std::string suppressTypeString =
comment.substr(pos1 + cppchecksuppress.size() + 1, argumentLength);

if ("file" == suppressTypeString) {
Expand All @@ -125,7 +125,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
} else if ("end" == suppressTypeString) {
errorType = Suppressions::Type::blockEnd;
} else {
return false;
return false;
}
}

Expand All @@ -136,7 +136,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:

for (Suppressions::Suppression &s : suppressions) {
s.type = errorType;
s.lineNumber = tok->location.line;
s.lineNumber = tok->location.line;
}

if (!errmsg.empty())
Expand All @@ -153,7 +153,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
return false;

s.type = errorType;
s.lineNumber = tok->location.line;
s.lineNumber = tok->location.line;

if (!s.errorId.empty())
inlineSuppressions.push_back(std::move(s));
Expand Down Expand Up @@ -196,6 +196,10 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
if (inlineSuppressions.empty())
continue;

// It should never happen
if (!tok)
continue;

// Relative filename
std::string relativeFilename(tok->location.file());
if (settings.relativePaths) {
Expand All @@ -217,15 +221,15 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
inlineSuppressionsBlockBegin.push_back(std::move(suppr));
} else if (Suppressions::Type::blockEnd == suppr.type) {
bool throwError = true;
if (inlineSuppressionsBlockBegin.size() > 0) {

if (!inlineSuppressionsBlockBegin.empty()) {
const Suppressions::Suppression lastBeginSuppression = inlineSuppressionsBlockBegin.back();

for (Suppressions::Suppression &supprBegin : inlineSuppressionsBlockBegin)
for (const Suppressions::Suppression &supprBegin : inlineSuppressionsBlockBegin)
{
if (lastBeginSuppression.lineNumber != supprBegin.lineNumber)
continue;

if (suppr.symbolName == supprBegin.symbolName && suppr.lineNumber > supprBegin.lineNumber) {
suppr.lineBegin = supprBegin.lineNumber;
suppr.lineEnd = suppr.lineNumber;
Expand All @@ -238,38 +242,33 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
}
}
}

if (throwError) {
// NOLINTNEXTLINE(bugprone-use-after-move) - moved only when thrownError is false
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress End: No matching begin");
}
} else if (Suppressions::Type::unique == suppr.type) {
if (!tok) {
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Unique suppress without matching code");
} else {
// special handling when suppressing { warnings for backwards compatibility
const bool thisAndNextLine = tok->previous &&
tok->previous->previous &&
tok->next &&
!sameline(tok->previous->previous, tok->previous) &&
tok->location.line + 1 == tok->next->location.line &&
tok->location.fileIndex == tok->next->location.fileIndex &&
tok->previous->str() == "{";

suppr.thisAndNextLine = thisAndNextLine;
suppr.lineNumber = tok->location.line;
suppressions.addSuppression(std::move(suppr));
}
// special handling when suppressing { warnings for backwards compatibility
const bool thisAndNextLine = tok->previous &&
tok->previous->previous &&
tok->next &&
!sameline(tok->previous->previous, tok->previous) &&
tok->location.line + 1 == tok->next->location.line &&
tok->location.fileIndex == tok->next->location.fileIndex &&
tok->previous->str() == "{";

suppr.thisAndNextLine = thisAndNextLine;
suppr.lineNumber = tok->location.line;
suppressions.addSuppression(std::move(suppr));
} else {
suppressions.addSuppression(std::move(suppr));
}
}
}

if (inlineSuppressionsBlockBegin.size() > 0) {
for (Suppressions::Suppression &suppr : inlineSuppressionsBlockBegin) {
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress Begin: No matching end");
}
}
std::for_each(inlineSuppressionsBlockBegin.begin(), inlineSuppressionsBlockBegin.end(), [&](const Suppressions::Suppression & suppr) {
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress Begin: No matching end");
});
}

void Preprocessor::inlineSuppressions(const simplecpp::TokenList &tokens, Suppressions &suppressions)
Expand Down
4 changes: 2 additions & 2 deletions lib/suppressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ bool Suppressions::Suppression::parseComment(std::string comment, std::string *e

if (comment.compare(comment.size() - 2, 2, "*/") == 0)
comment.erase(comment.size() - 2, 2);

const std::string cppchecksuppress = "cppcheck-suppress";

std::istringstream iss(comment.substr(2));
Expand Down Expand Up @@ -486,7 +486,7 @@ void Suppressions::markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &tok
suppression.checked = true;
}
} else if (suppression.type == Suppressions::Type::block) {
if ((!suppression.checked && (suppression.lineBegin <= currLineNr) && (suppression.lineEnd >= currLineNr) && suppression.fileName == tokenizer.list.file(tok))){
if ((!suppression.checked && (suppression.lineBegin <= currLineNr) && (suppression.lineEnd >= currLineNr) && suppression.fileName == tokenizer.list.file(tok))) {
suppression.checked = true;
}
} else if (!suppression.checked && suppression.fileName == tokenizer.list.file(tok)) {
Expand Down
3 changes: 1 addition & 2 deletions test/testsuppressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,6 @@ class TestSuppressions : public TestFixture {
"");
ASSERT_EQUALS("[test.cpp:4]: (information) Unmatched suppression: uninitvar\n", errout.str());


// suppress block inline checks
ASSERT_EQUALS(0, (this->*check)("void f() {\n"
" // cppcheck-suppress-begin uninitvar\n"
Expand Down Expand Up @@ -632,7 +631,7 @@ class TestSuppressions : public TestFixture {
"}\n",
"");
ASSERT_EQUALS("", errout.str());

(this->*check)("void f() {\n"
" // cppcheck-suppress-begin [uninitvar]\n"
" int a;\n"
Expand Down

0 comments on commit 1ed3d98

Please sign in to comment.