Skip to content

Commit 29e9866

Browse files
firewavefirewave
committed
fixed #14265 - fixed error location for several preprocessor errors [skip ci]
1 parent 61a4ae9 commit 29e9866

6 files changed

Lines changed: 93 additions & 70 deletions

File tree

‎cli/cppcheckexecutor.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ static bool reportUnmatchedSuppressions(const std::list<SuppressionList::Suppres
327327

328328
std::list<::ErrorMessage::FileLocation> callStack;
329329
if (!s.fileName.empty()) {
330-
callStack.emplace_back(s.fileName, s.lineNumber, 0);
330+
callStack.emplace_back(s.fileName, s.lineNumber, 0); // TODO: set column - see #13810
331331
}
332332
errorLogger.reportErr(::ErrorMessage(std::move(callStack), "", Severity::information, "Unmatched suppression: " + s.errorId, "unmatchedSuppression", Certainty::normal));
333333
err = true;

‎lib/cppcheck.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1162,7 +1162,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
11621162

11631163
if (!hasValidConfig && currCfg == *configurations.rbegin()) {
11641164
// If there is no valid configuration then report error..
1165-
preprocessor.error(tokensP.file(o->location), o->location.line, o->location.col, o->msg, o->type);
1165+
preprocessor.error(o->location, o->msg, o->type);
11661166
}
11671167
skipCfg = true;
11681168
}

‎lib/preprocessor.cpp‎

Lines changed: 62 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,13 @@ Preprocessor::Preprocessor(simplecpp::TokenList& tokens, const Settings& setting
7171

7272
namespace {
7373
struct BadInlineSuppression {
74-
BadInlineSuppression(std::string file, const int line, unsigned int col, std::string msg) : file(std::move(file)), line(line), col(col), errmsg(std::move(msg)) {}
75-
std::string file;
76-
int line; // TODO: needs to be unsigned
77-
unsigned int col;
74+
BadInlineSuppression(const simplecpp::Location& loc, std::string msg) : location(loc), errmsg(std::move(msg)) {}
75+
simplecpp::Location location;
7876
std::string errmsg;
7977
};
8078
}
8179

82-
static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &tokens, const simplecpp::Token *tok, std::list<SuppressionList::Suppression> &inlineSuppressions, std::list<BadInlineSuppression> &bad)
80+
static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std::list<SuppressionList::Suppression> &inlineSuppressions, std::list<BadInlineSuppression> &bad)
8381
{
8482
const std::string cppchecksuppress("cppcheck-suppress");
8583

@@ -92,7 +90,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
9290
if (comment.substr(pos1, cppchecksuppress.size()) != cppchecksuppress)
9391
return false;
9492
if (pos1 + cppchecksuppress.size() >= comment.size()) {
95-
bad.emplace_back(tokens.file(tok->location), tok->location.line, 0, "suppression without error ID");
93+
bad.emplace_back(tok->location, "suppression without error ID");
9694
return false;
9795
}
9896

@@ -102,7 +100,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
102100
// skip spaces after "cppcheck-suppress" and its possible prefix
103101
const std::string::size_type pos2 = comment.find_first_not_of(' ', posEndComment);
104102
if (pos2 == std::string::npos) {
105-
bad.emplace_back(tokens.file(tok->location), tok->location.line, 0, "suppression without error ID");
103+
bad.emplace_back(tok->location, "suppression without error ID");
106104
return false;
107105
}
108106

@@ -112,7 +110,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
112110
if (posEndComment >= (pos1 + cppchecksuppress.size() + 1)) {
113111
const std::string suppressCmdString = comment.substr(pos1, pos2-pos1-1);
114112
if (comment.at(pos1 + cppchecksuppress.size()) != '-') {
115-
bad.emplace_back(tokens.file(tok->location), tok->location.line, 0, "unknown suppression type '" + suppressCmdString + "'"); // TODO: set column
113+
bad.emplace_back(tok->location, "unknown suppression type '" + suppressCmdString + "'");
116114
return false;
117115
}
118116

@@ -131,7 +129,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
131129
else if ("macro" == suppressTypeString)
132130
errorType = SuppressionList::Type::macro;
133131
else {
134-
bad.emplace_back(tokens.file(tok->location), tok->location.line, 0, "unknown suppression type '" + suppressCmdString + "'"); // TODO: set column
132+
bad.emplace_back(tok->location, "unknown suppression type '" + suppressCmdString + "'");
135133
return false;
136134
}
137135
}
@@ -145,11 +143,12 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
145143
s.isInline = true;
146144
s.type = errorType;
147145
s.lineNumber = tok->location.line;
146+
s.column = tok->location.col;
148147
}
149148

150149
// TODO: return false?
151150
if (!errmsg.empty())
152-
bad.emplace_back(tokens.file(tok->location), tok->location.line, tok->location.col, std::move(errmsg));
151+
bad.emplace_back(tok->location, std::move(errmsg));
153152

154153
// TODO: report ones without ID - return false?
155154
std::copy_if(suppressions.cbegin(), suppressions.cend(), std::back_inserter(inlineSuppressions), [](const SuppressionList::Suppression& s) {
@@ -165,6 +164,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
165164
s.isInline = true;
166165
s.type = errorType;
167166
s.lineNumber = tok->location.line;
167+
s.column = tok->location.col;
168168

169169
// TODO: report when no ID - unreachable?
170170
if (!s.errorId.empty())
@@ -173,7 +173,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::TokenList &token
173173
// TODO: unreachable?
174174
// TODO: return false?
175175
if (!errmsg.empty())
176-
bad.emplace_back(tokens.file(tok->location), tok->location.line, tok->location.col, std::move(errmsg));
176+
bad.emplace_back(tok->location, std::move(errmsg));
177177
}
178178

179179
return true;
@@ -207,7 +207,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
207207
}
208208

209209
std::list<SuppressionList::Suppression> inlineSuppressions;
210-
if (!parseInlineSuppressionCommentToken(tokens, tok, inlineSuppressions, bad))
210+
if (!parseInlineSuppressionCommentToken(tok, inlineSuppressions, bad))
211211
continue;
212212

213213
if (!sameline(tok->previous, tok)) {
@@ -216,7 +216,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
216216
tok = tok->next;
217217

218218
while (tok->comment) {
219-
parseInlineSuppressionCommentToken(tokens, tok, inlineSuppressions, bad);
219+
parseInlineSuppressionCommentToken(tok, inlineSuppressions, bad);
220220
if (tok->next) {
221221
tok = tok->next;
222222
} else {
@@ -248,6 +248,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
248248
// Add the suppressions.
249249
for (SuppressionList::Suppression &suppr : inlineSuppressions) {
250250
suppr.fileName = relativeFilename;
251+
suppr.fileIndex = tok->location.fileIndex;
251252

252253
if (SuppressionList::Type::blockBegin == suppr.type)
253254
{
@@ -270,6 +271,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
270271
suppr.lineBegin = supprBegin->lineNumber;
271272
suppr.lineEnd = suppr.lineNumber;
272273
suppr.lineNumber = supprBegin->lineNumber;
274+
suppr.column = supprBegin->column;
273275
suppr.type = SuppressionList::Type::block;
274276
inlineSuppressionsBlockBegin.erase(supprBegin);
275277
suppressions.addSuppression(std::move(suppr)); // TODO: check result
@@ -281,8 +283,12 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
281283
}
282284

283285
if (throwError) {
286+
simplecpp::Location loc;
284287
// NOLINTNEXTLINE(bugprone-use-after-move) - moved only when thrownError is false
285-
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "Suppress End: No matching begin"); // TODO: set column
288+
loc.fileIndex = suppr.fileIndex;
289+
loc.line = suppr.lineNumber;
290+
loc.col = suppr.column;
291+
bad.emplace_back(loc, "Suppress End: No matching begin");
286292
}
287293
} else if (SuppressionList::Type::unique == suppr.type || suppr.type == SuppressionList::Type::macro) {
288294
// special handling when suppressing { warnings for backwards compatibility
@@ -296,20 +302,31 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
296302

297303
suppr.thisAndNextLine = thisAndNextLine;
298304
suppr.lineNumber = tok->location.line;
305+
suppr.column = tok->location.col;
299306
suppr.macroName = macroName;
300307
suppressions.addSuppression(std::move(suppr)); // TODO: check result
301308
} else if (SuppressionList::Type::file == suppr.type) {
302309
if (onlyComments)
303310
suppressions.addSuppression(std::move(suppr)); // TODO: check result
304-
else
305-
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "File suppression should be at the top of the file"); // TODO: set column
311+
else {
312+
simplecpp::Location loc;
313+
loc.fileIndex = suppr.fileIndex;
314+
loc.line = suppr.lineNumber;
315+
loc.col = suppr.column;
316+
bad.emplace_back(loc, "File suppression should be at the top of the file");
317+
}
306318
}
307319
}
308320
}
309321

310-
for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin)
322+
for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin) {
323+
simplecpp::Location loc;
324+
loc.fileIndex = suppr.fileIndex;
325+
loc.line = suppr.lineNumber;
326+
loc.col = suppr.column;
311327
// cppcheck-suppress useStlAlgorithm
312-
bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "Suppress Begin: No matching end"); // TODO: set column
328+
bad.emplace_back(loc, "Suppress Begin: No matching end");
329+
}
313330
}
314331

315332
void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
@@ -322,7 +339,7 @@ void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
322339
::addInlineSuppressions(filedata->tokens, mSettings, suppressions, err);
323340
}
324341
for (const BadInlineSuppression &bad : err) {
325-
invalidSuppression(bad.file, bad.line, bad.col, bad.errmsg); // TODO: column is always 0
342+
invalidSuppression(bad.location, bad.errmsg);
326343
}
327344
}
328345

@@ -865,7 +882,7 @@ const simplecpp::Output* Preprocessor::reportOutput(const simplecpp::OutputList
865882
case simplecpp::Output::ERROR:
866883
out_ret = &out;
867884
if (!startsWith(out.msg,"#error") || showerror)
868-
error(mTokens.file(out.location), out.location.line, out.location.col, out.msg, out.type);
885+
error(out.location, out.msg, out.type);
869886
break;
870887
case simplecpp::Output::WARNING:
871888
case simplecpp::Output::PORTABILITY_BACKSLASH:
@@ -875,20 +892,20 @@ const simplecpp::Output* Preprocessor::reportOutput(const simplecpp::OutputList
875892
const std::string::size_type pos1 = out.msg.find_first_of("<\"");
876893
const std::string::size_type pos2 = out.msg.find_first_of(">\"", pos1 + 1U);
877894
if (pos1 < pos2 && pos2 != std::string::npos)
878-
missingInclude(mTokens.file(out.location), out.location.line, out.location.col, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
895+
missingInclude(out.location, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
879896
}
880897
break;
881898
case simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY:
882899
case simplecpp::Output::SYNTAX_ERROR:
883900
case simplecpp::Output::UNHANDLED_CHAR_ERROR:
884901
out_ret = &out;
885-
error(mTokens.file(out.location), out.location.line, out.location.col, out.msg, out.type);
902+
error(out.location, out.msg, out.type);
886903
break;
887904
case simplecpp::Output::EXPLICIT_INCLUDE_NOT_FOUND:
888905
case simplecpp::Output::FILE_NOT_FOUND:
889906
case simplecpp::Output::DUI_ERROR:
890907
out_ret = &out;
891-
error("", 0, 0, out.msg, out.type);
908+
error({}, out.msg, out.type);
892909
break;
893910
}
894911
}
@@ -923,20 +940,20 @@ static std::string simplecppErrToId(simplecpp::Output::Type type)
923940
cppcheck::unreachable();
924941
}
925942

926-
void Preprocessor::error(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, simplecpp::Output::Type type)
943+
void Preprocessor::error(const simplecpp::Location& loc, const std::string &msg, simplecpp::Output::Type type)
927944
{
928-
error(filename, linenr, col, msg, simplecppErrToId(type));
945+
error(loc, msg, simplecppErrToId(type));
929946
}
930947

931-
void Preprocessor::error(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, const std::string& id)
948+
void Preprocessor::error(const simplecpp::Location& loc, const std::string &msg, const std::string& id)
932949
{
933950
std::list<ErrorMessage::FileLocation> locationList;
934-
if (!filename.empty()) {
935-
std::string file = Path::fromNativeSeparators(filename);
951+
if (!mTokens.file(loc).empty()) {
952+
std::string file = Path::fromNativeSeparators(mTokens.file(loc));
936953
if (mSettings.relativePaths)
937954
file = Path::getRelativePath(file, mSettings.basePaths);
938955

939-
locationList.emplace_back(file, linenr, col);
956+
locationList.emplace_back(file, loc.line, loc.col);
940957
}
941958
mErrorLogger.reportErr(ErrorMessage(std::move(locationList),
942959
mFile0,
@@ -947,15 +964,15 @@ void Preprocessor::error(const std::string &filename, unsigned int linenr, unsig
947964
}
948965

949966
// Report that include is missing
950-
void Preprocessor::missingInclude(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &header, HeaderTypes headerType)
967+
void Preprocessor::missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType)
951968
{
952969
if (!mSettings.checks.isEnabled(Checks::missingInclude))
953970
return;
954971

955972
std::list<ErrorMessage::FileLocation> locationList;
956-
if (!filename.empty()) {
973+
if (!mTokens.file(loc).empty()) {
957974
// TODO: add relative path handling?
958-
locationList.emplace_back(filename, linenr, col);
975+
locationList.emplace_back(mTokens.file(loc), loc.line, loc.col);
959976
}
960977
ErrorMessage errmsg(std::move(locationList), mFile0, Severity::information,
961978
(headerType==SystemHeader) ?
@@ -966,24 +983,27 @@ void Preprocessor::missingInclude(const std::string &filename, unsigned int line
966983
mErrorLogger.reportErr(errmsg);
967984
}
968985

969-
void Preprocessor::invalidSuppression(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg)
986+
void Preprocessor::invalidSuppression(const simplecpp::Location& loc, const std::string &msg)
970987
{
971-
error(filename, linenr, col, msg, "invalidSuppression");
988+
error(loc, msg, "invalidSuppression");
972989
}
973990

974991
void Preprocessor::getErrorMessages(ErrorLogger &errorLogger, const Settings &settings)
975992
{
976993
std::vector<std::string> files;
977994
simplecpp::TokenList tokens(files);
978995
Preprocessor preprocessor(tokens, settings, errorLogger, Standards::Language::CPP);
979-
preprocessor.missingInclude("", 1, 2, "", UserHeader);
980-
preprocessor.missingInclude("", 1, 2, "", SystemHeader);
981-
preprocessor.error("", 1, 2, "message", simplecpp::Output::ERROR);
982-
preprocessor.error("", 1, 2, "message", simplecpp::Output::SYNTAX_ERROR);
983-
preprocessor.error("", 1, 2, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
984-
preprocessor.error("", 1, 2, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
985-
preprocessor.error("", 1, 2, "message", simplecpp::Output::FILE_NOT_FOUND);
986-
preprocessor.invalidSuppression("", 1, 2, "message");
996+
simplecpp::Location loc;
997+
loc.line = 1;
998+
loc.col = 2;
999+
preprocessor.missingInclude(loc, "", UserHeader);
1000+
preprocessor.missingInclude(loc, "", SystemHeader);
1001+
preprocessor.error(loc, "message", simplecpp::Output::ERROR);
1002+
preprocessor.error(loc, "message", simplecpp::Output::SYNTAX_ERROR);
1003+
preprocessor.error(loc, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
1004+
preprocessor.error(loc, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
1005+
preprocessor.error(loc, "message", simplecpp::Output::FILE_NOT_FOUND);
1006+
preprocessor.invalidSuppression(loc, "message");
9871007
}
9881008

9891009
void Preprocessor::dump(std::ostream &out) const

‎lib/preprocessor.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
141141

142142
const simplecpp::Output* reportOutput(const simplecpp::OutputList &outputList, bool showerror);
143143

144-
void error(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, simplecpp::Output::Type type);
144+
void error(const simplecpp::Location& loc, const std::string &msg, simplecpp::Output::Type type);
145145

146146
const simplecpp::Output* handleErrors(const simplecpp::OutputList &outputList);
147147

@@ -156,9 +156,9 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
156156
SystemHeader
157157
};
158158

159-
void missingInclude(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &header, HeaderTypes headerType);
160-
void invalidSuppression(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg);
161-
void error(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &msg, const std::string& id);
159+
void missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType);
160+
void invalidSuppression(const simplecpp::Location& loc, const std::string &msg);
161+
void error(const simplecpp::Location& loc, const std::string &msg, const std::string& id);
162162

163163
void addRemarkComments(const simplecpp::TokenList &tokens, std::vector<RemarkComment> &remarkComments) const;
164164

‎lib/suppressions.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,12 @@ class CPPCHECKLIB SuppressionList {
149149
std::string errorId;
150150
std::string fileName;
151151
std::string extraComment;
152+
// TODO: use simplecpp::Location?
153+
int fileIndex{};
152154
int lineNumber = NO_LINE; // TODO: needs to be unsigned
153155
int lineBegin = NO_LINE;
154156
int lineEnd = NO_LINE;
157+
int column{};
155158
Type type = Type::unique;
156159
std::string symbolName;
157160
std::string macroName;

0 commit comments

Comments
 (0)