Skip to content

Commit

Permalink
Fixed danmar#3537 (Allow inline suppression comments for macros) (dan…
Browse files Browse the repository at this point in the history
  • Loading branch information
danmar authored Oct 16, 2023
1 parent ad4e688 commit dd76504
Show file tree
Hide file tree
Showing 14 changed files with 152 additions and 30 deletions.
2 changes: 1 addition & 1 deletion cli/executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Executor::Executor(const std::map<std::string, std::size_t> &files, const Settin

bool Executor::hasToLog(const ErrorMessage &msg)
{
if (!mSuppressions.isSuppressed(msg))
if (!mSuppressions.isSuppressed(msg, {}))
{
std::string errmsg = msg.toString(mSettings.verbose);

Expand Down
2 changes: 1 addition & 1 deletion cli/processexecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ void ProcessExecutor::reportInternalChildErr(const std::string &childname, const
"cppcheckError",
Certainty::normal);

if (!mSuppressions.isSuppressed(errmsg))
if (!mSuppressions.isSuppressed(errmsg, {}))
mErrorLogger.reportErr(errmsg);
}

Expand Down
18 changes: 17 additions & 1 deletion lib/cppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,13 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
}
hasValidConfig = true;

// locations macros
mLocationMacros.clear();
for (const Token* tok = tokenizer.tokens(); tok; tok = tok->next()) {
if (!tok->getMacroName().empty())
mLocationMacros[Location(files[tok->fileIndex()], tok->linenr())].emplace(tok->getMacroName());
}

// If only errors are printed, print filename after the check
if (!mSettings.quiet && (!mCurrentConfig.empty() || checkCount > 1)) {
std::string fixedpath = Path::simplifyPath(filename);
Expand Down Expand Up @@ -1556,8 +1563,17 @@ void CppCheck::reportErr(const ErrorMessage &msg)
if (!mSettings.buildDir.empty())
mAnalyzerInformation.reportErr(msg);

std::set<std::string> macroNames;
if (!msg.callStack.empty()) {
const std::string &file = msg.callStack.back().getfile(false);
int lineNumber = msg.callStack.back().line;
const auto it = mLocationMacros.find(Location(file, lineNumber));
if (it != mLocationMacros.cend())
macroNames = it->second;
}

// TODO: only convert if necessary
const auto errorMessage = Suppressions::ErrorMessage::fromErrorMessage(msg);
const auto errorMessage = Suppressions::ErrorMessage::fromErrorMessage(msg, macroNames);

if (mSettings.nomsg.isSuppressed(errorMessage, mUseGlobalSuppressions)) {
return;
Expand Down
4 changes: 4 additions & 0 deletions lib/cppcheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <functional>
#include <list>
#include <map>
#include <set>
#include <string>
#include <vector>

Expand Down Expand Up @@ -223,6 +224,9 @@ class CPPCHECKLIB CppCheck : ErrorLogger {
/** @brief Current preprocessor configuration */
std::string mCurrentConfig;

using Location = std::pair<std::string, int>;
std::map<Location, std::set<std::string>> mLocationMacros; // What macros are used on a location?

unsigned int mExitCode{};

bool mUseGlobalSuppressions;
Expand Down
23 changes: 17 additions & 6 deletions lib/preprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,16 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
const std::string suppressTypeString =
comment.substr(pos1 + cppchecksuppress.size() + 1, argumentLength);

if ("file" == suppressTypeString) {
if ("file" == suppressTypeString)
errorType = Suppressions::Type::file;
} else if ("begin" == suppressTypeString) {
else if ("begin" == suppressTypeString)
errorType = Suppressions::Type::blockBegin;
} else if ("end" == suppressTypeString) {
else if ("end" == suppressTypeString)
errorType = Suppressions::Type::blockEnd;
} else {
else if ("macro" == suppressTypeString)
errorType = Suppressions::Type::macro;
else
return false;
}
}

if (comment[pos2] == '[') {
Expand Down Expand Up @@ -217,6 +218,15 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
}
relativeFilename = Path::simplifyPath(relativeFilename);

// Macro name
std::string macroName;
if (tok->str() == "#" && tok->next && tok->next->str() == "define") {
const simplecpp::Token *macroNameTok = tok->next->next;
if (sameline(tok, macroNameTok) && macroNameTok->name) {
macroName = macroNameTok->str();
}
}

// Add the suppressions.
for (Suppressions::Suppression &suppr : inlineSuppressions) {
suppr.fileName = relativeFilename;
Expand Down Expand Up @@ -252,7 +262,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
// 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) {
} else if (Suppressions::Type::unique == suppr.type || suppr.type == Suppressions::Type::macro) {
// special handling when suppressing { warnings for backwards compatibility
const bool thisAndNextLine = tok->previous &&
tok->previous->previous &&
Expand All @@ -264,6 +274,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett

suppr.thisAndNextLine = thisAndNextLine;
suppr.lineNumber = tok->location.line;
suppr.macroName = macroName;
suppressions.addSuppression(std::move(suppr));
} else if (Suppressions::Type::file == suppr.type) {
if (onlyComments)
Expand Down
29 changes: 19 additions & 10 deletions lib/suppressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

#include <tinyxml2.h>

Suppressions::ErrorMessage Suppressions::ErrorMessage::fromErrorMessage(const ::ErrorMessage &msg)
Suppressions::ErrorMessage Suppressions::ErrorMessage::fromErrorMessage(const ::ErrorMessage &msg, const std::set<std::string> &macroNames)
{
Suppressions::ErrorMessage ret;
ret.hash = msg.hash;
Expand All @@ -48,6 +48,7 @@ Suppressions::ErrorMessage Suppressions::ErrorMessage::fromErrorMessage(const ::
}
ret.certainty = msg.certainty;
ret.symbolNames = msg.symbolNames();
ret.macroNames = macroNames;
return ret;
}

Expand Down Expand Up @@ -308,7 +309,8 @@ bool Suppressions::Suppression::parseComment(std::string comment, std::string *e
"cppcheck-suppress",
"cppcheck-suppress-begin",
"cppcheck-suppress-end",
"cppcheck-suppress-file"
"cppcheck-suppress-file",
"cppcheck-suppress-macro"
};

std::istringstream iss(comment.substr(2));
Expand Down Expand Up @@ -343,14 +345,19 @@ bool Suppressions::Suppression::isSuppressed(const Suppressions::ErrorMessage &e
return false;
if (!errorId.empty() && !matchglob(errorId, errmsg.errorId))
return false;
if (!fileName.empty() && !matchglob(fileName, errmsg.getFileName()))
return false;
if ((Suppressions::Type::unique == type) && (lineNumber != NO_LINE) && (lineNumber != errmsg.lineNumber)) {
if (!thisAndNextLine || lineNumber + 1 != errmsg.lineNumber)
if (type == Suppressions::Type::macro) {
if (errmsg.macroNames.count(macroName) == 0)
return false;
} else {
if (!fileName.empty() && !matchglob(fileName, errmsg.getFileName()))
return false;
if ((Suppressions::Type::unique == type) && (lineNumber != NO_LINE) && (lineNumber != errmsg.lineNumber)) {
if (!thisAndNextLine || lineNumber + 1 != errmsg.lineNumber)
return false;
}
if ((Suppressions::Type::block == type) && ((errmsg.lineNumber < lineBegin) || (errmsg.lineNumber > lineEnd)))
return false;
}
if ((Suppressions::Type::block == type) && ((errmsg.lineNumber < lineBegin) || (errmsg.lineNumber > lineEnd)))
return false;
if (!symbolName.empty()) {
for (std::string::size_type pos = 0; pos < errmsg.symbolNames.size();) {
const std::string::size_type pos2 = errmsg.symbolNames.find('\n',pos);
Expand Down Expand Up @@ -412,11 +419,11 @@ bool Suppressions::isSuppressed(const Suppressions::ErrorMessage &errmsg, bool g
return returnValue;
}

bool Suppressions::isSuppressed(const ::ErrorMessage &errmsg)
bool Suppressions::isSuppressed(const ::ErrorMessage &errmsg, const std::set<std::string>& macroNames)
{
if (mSuppressions.empty())
return false;
return isSuppressed(Suppressions::ErrorMessage::fromErrorMessage(errmsg));
return isSuppressed(Suppressions::ErrorMessage::fromErrorMessage(errmsg, macroNames));
}

void Suppressions::dump(std::ostream & out) const
Expand Down Expand Up @@ -445,6 +452,8 @@ std::list<Suppressions::Suppression> Suppressions::getUnmatchedLocalSuppressions
for (const Suppression &s : mSuppressions) {
if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked))
continue;
if (s.type == Suppressions::Type::macro)
continue;
if (s.hash > 0)
continue;
if (!unusedFunctionChecking && s.errorId == "unusedFunction")
Expand Down
13 changes: 10 additions & 3 deletions lib/suppressions.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <cstddef>
#include <istream>
#include <list>
#include <set>
#include <string>
#include <utility>
#include <vector>
Expand All @@ -42,7 +43,7 @@ class CPPCHECKLIB Suppressions {
public:

enum class Type {
unique, file, block, blockBegin, blockEnd
unique, file, block, blockBegin, blockEnd, macro
};

struct CPPCHECKLIB ErrorMessage {
Expand All @@ -55,8 +56,9 @@ class CPPCHECKLIB Suppressions {
int lineNumber;
Certainty certainty;
std::string symbolNames;
std::set<std::string> macroNames;

static Suppressions::ErrorMessage fromErrorMessage(const ::ErrorMessage &msg);
static Suppressions::ErrorMessage fromErrorMessage(const ::ErrorMessage &msg, const std::set<std::string> &macroNames);
private:
std::string mFileName;
};
Expand All @@ -74,6 +76,8 @@ class CPPCHECKLIB Suppressions {
return fileName < other.fileName;
if (symbolName != other.symbolName)
return symbolName < other.symbolName;
if (macroName != other.macroName)
return macroName < other.macroName;
if (hash != other.hash)
return hash < other.hash;
if (thisAndNextLine != other.thisAndNextLine)
Expand All @@ -90,6 +94,8 @@ class CPPCHECKLIB Suppressions {
return false;
if (symbolName != other.symbolName)
return false;
if (macroName != other.macroName)
return false;
if (hash != other.hash)
return false;
if (type != other.type)
Expand Down Expand Up @@ -135,6 +141,7 @@ class CPPCHECKLIB Suppressions {
int lineEnd = NO_LINE;
Type type = Type::unique;
std::string symbolName;
std::string macroName;
std::size_t hash{};
bool thisAndNextLine{}; // Special case for backwards compatibility: { // cppcheck-suppress something
bool matched{};
Expand Down Expand Up @@ -200,7 +207,7 @@ class CPPCHECKLIB Suppressions {
* @param errmsg error message
* @return true if this error is suppressed.
*/
bool isSuppressed(const ::ErrorMessage &errmsg);
bool isSuppressed(const ::ErrorMessage &errmsg, const std::set<std::string>& macroNames);

/**
* @brief Create an xml dump of suppressions
Expand Down
17 changes: 12 additions & 5 deletions lib/token.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ struct TokenImpl {
// original name like size_t
std::string* mOriginalName{};

// If this token came from a macro replacement list, this is the name of that macro
std::string mMacroName;

// ValueType
ValueType* mValueType{};

Expand Down Expand Up @@ -461,10 +464,7 @@ class CPPCHECKLIB Token {
setFlag(fIsStandardType, b);
}
bool isExpandedMacro() const {
return getFlag(fIsExpandedMacro);
}
void isExpandedMacro(const bool m) {
setFlag(fIsExpandedMacro, m);
return !mImpl->mMacroName.empty();
}
bool isCast() const {
return getFlag(fIsCast);
Expand Down Expand Up @@ -763,6 +763,13 @@ class CPPCHECKLIB Token {
setFlag(fIsTemplateArg, value);
}

std::string getMacroName() const {
return mImpl->mMacroName;
}
void setMacroName(std::string name) {
mImpl->mMacroName = std::move(name);
}

template<size_t count>
static const Token *findsimplematch(const Token * const startTok, const char (&pattern)[count]) {
return findsimplematch(startTok, pattern, count-1);
Expand Down Expand Up @@ -1305,7 +1312,7 @@ class CPPCHECKLIB Token {
fIsPointerCompare = (1ULL << 2),
fIsLong = (1ULL << 3),
fIsStandardType = (1ULL << 4),
fIsExpandedMacro = (1ULL << 5),
//fIsExpandedMacro = (1ULL << 5),
fIsCast = (1ULL << 6),
fIsAttributeConstructor = (1ULL << 7), // __attribute__((constructor)) __attribute__((constructor(priority)))
fIsAttributeDestructor = (1ULL << 8), // __attribute__((destructor)) __attribute__((destructor(priority)))
Expand Down
4 changes: 2 additions & 2 deletions lib/tokenize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ namespace {
if (pointerType) {
tok->insertToken("const");
tok->next()->column(tok->column());
tok->next()->isExpandedMacro(tok->previous()->isExpandedMacro());
tok->next()->setMacroName(tok->previous()->getMacroName());
tok->deletePrevious();
}
}
Expand Down Expand Up @@ -7169,7 +7169,7 @@ void Tokenizer::simplifyVarDecl(Token * tokBegin, const Token * const tokEnd, co
endDecl = endDecl->next();
endDecl->next()->isSplittedVarDeclEq(true);
endDecl->insertToken(varName->str());
endDecl->next()->isExpandedMacro(varName->isExpandedMacro());
endDecl->next()->setMacroName(varName->getMacroName());
continue;
}
//non-VLA case
Expand Down
3 changes: 2 additions & 1 deletion lib/tokenlist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ void TokenList::insertTokens(Token *dest, const Token *src, nonneg int n)
dest->varId(src->varId());
dest->tokType(src->tokType());
dest->flags(src->flags());
dest->setMacroName(src->getMacroName());
src = src->next();
--n;
}
Expand Down Expand Up @@ -363,7 +364,7 @@ void TokenList::createTokens(simplecpp::TokenList&& tokenList)
mTokensFrontBack.back->fileIndex(tok->location.fileIndex);
mTokensFrontBack.back->linenr(tok->location.line);
mTokensFrontBack.back->column(tok->location.col);
mTokensFrontBack.back->isExpandedMacro(!tok->macro.empty());
mTokensFrontBack.back->setMacroName(tok->macro);

tok = tok->next;
if (tok)
Expand Down
35 changes: 35 additions & 0 deletions man/manual-premium.md
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,41 @@ Suppressing multiple ids in one comment by using []:

// cppcheck-suppress [aaaa, bbbb]

Suppressing warnings `aaaa` on a block of code:

// cppcheck-suppress-begin aaaa
...
// cppcheck-suppress-end aaaa

Suppressing multiple ids on a block of code:

// cppcheck-suppress-begin [aaaa, bbbb]
...
// cppcheck-suppress-end [aaaa, bbbb]

Suppressing warnings `aaaa` for a whole file:

// cppcheck-suppress-file aaaa

Suppressing multiple ids for a whole file:

// cppcheck-suppress-file [aaaa, bbbb]

Suppressing warnings `aaaa` where macro is used:

// cppcheck-suppress-macro aaaa
#define MACRO ...
...
x = MACRO; // <- aaaa warnings are suppressed here


Suppressing multiple ids where macro is used:

// cppcheck-suppress-macro [aaaa, bbbb]
#define MACRO ...
...
x = MACRO; // <- aaaa and bbbb warnings are suppressed here

### Comment before code or on same line

The comment can be put before the code or at the same line as the code.
Expand Down
Loading

0 comments on commit dd76504

Please sign in to comment.