Skip to content

Commit 90c3fdc

Browse files
firewavefirewave
committed
fixed #14265 - fixed error location for several preprocessor errors [skip ci]
1 parent d8b6e08 commit 90c3fdc

File tree

6 files changed

+65
-44
lines changed

6 files changed

+65
-44
lines changed

‎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/preprocessor.cpp‎

Lines changed: 50 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,8 @@ Preprocessor::Preprocessor(simplecpp::TokenList& tokens, const Settings& setting
7676

7777
namespace {
7878
struct BadInlineSuppression {
79-
BadInlineSuppression(std::string file, const int line, std::string msg) : file(std::move(file)), line(line), errmsg(std::move(msg)) {}
80-
std::string file;
81-
int line;
79+
BadInlineSuppression(const simplecpp::Location& loc, std::string msg) : location(loc), errmsg(std::move(msg)) {}
80+
simplecpp::Location location;
8281
std::string errmsg;
8382
};
8483
}
@@ -140,10 +139,11 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
140139
s.isInline = true;
141140
s.type = errorType;
142141
s.lineNumber = tok->location.line;
142+
s.column = tok->location.col;
143143
}
144144

145145
if (!errmsg.empty())
146-
bad.emplace_back(tok->location.file(), tok->location.line, std::move(errmsg));
146+
bad.emplace_back(tok->location, std::move(errmsg));
147147

148148
std::copy_if(suppressions.cbegin(), suppressions.cend(), std::back_inserter(inlineSuppressions), [](const SuppressionList::Suppression& s) {
149149
return !s.errorId.empty();
@@ -158,12 +158,13 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
158158
s.isInline = true;
159159
s.type = errorType;
160160
s.lineNumber = tok->location.line;
161+
s.column = tok->location.col;
161162

162163
if (!s.errorId.empty())
163164
inlineSuppressions.push_back(std::move(s));
164165

165166
if (!errmsg.empty())
166-
bad.emplace_back(tok->location.file(), tok->location.line, std::move(errmsg));
167+
bad.emplace_back(tok->location, std::move(errmsg));
167168
}
168169

169170
return true;
@@ -238,6 +239,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
238239
// Add the suppressions.
239240
for (SuppressionList::Suppression &suppr : inlineSuppressions) {
240241
suppr.fileName = relativeFilename;
242+
suppr.fileIndex = tok->location.fileIndex;
241243

242244
if (SuppressionList::Type::blockBegin == suppr.type)
243245
{
@@ -260,6 +262,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
260262
suppr.lineBegin = supprBegin->lineNumber;
261263
suppr.lineEnd = suppr.lineNumber;
262264
suppr.lineNumber = supprBegin->lineNumber;
265+
suppr.column = supprBegin->column;
263266
suppr.type = SuppressionList::Type::block;
264267
inlineSuppressionsBlockBegin.erase(supprBegin);
265268
suppressions.addSuppression(std::move(suppr)); // TODO: check result
@@ -271,8 +274,12 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
271274
}
272275

273276
if (throwError) {
277+
simplecpp::Location loc(tokens.getFiles());
274278
// NOLINTNEXTLINE(bugprone-use-after-move) - moved only when thrownError is false
275-
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress End: No matching begin");
279+
loc.fileIndex = suppr.fileIndex;
280+
loc.line = suppr.lineNumber;
281+
loc.col = suppr.column;
282+
bad.emplace_back(loc, "Suppress End: No matching begin");
276283
}
277284
} else if (SuppressionList::Type::unique == suppr.type || suppr.type == SuppressionList::Type::macro) {
278285
// special handling when suppressing { warnings for backwards compatibility
@@ -286,20 +293,31 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
286293

287294
suppr.thisAndNextLine = thisAndNextLine;
288295
suppr.lineNumber = tok->location.line;
296+
suppr.column = tok->location.col;
289297
suppr.macroName = macroName;
290298
suppressions.addSuppression(std::move(suppr)); // TODO: check result
291299
} else if (SuppressionList::Type::file == suppr.type) {
292300
if (onlyComments)
293301
suppressions.addSuppression(std::move(suppr)); // TODO: check result
294-
else
295-
bad.emplace_back(suppr.fileName, suppr.lineNumber, "File suppression should be at the top of the file");
302+
else {
303+
simplecpp::Location loc(tokens.getFiles());
304+
loc.fileIndex = suppr.fileIndex;
305+
loc.line = suppr.lineNumber;
306+
loc.col = suppr.column;
307+
bad.emplace_back(loc, "File suppression should be at the top of the file");
308+
}
296309
}
297310
}
298311
}
299312

300-
for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin)
313+
for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin) {
314+
simplecpp::Location loc(tokens.getFiles());
315+
loc.fileIndex = suppr.fileIndex;
316+
loc.line = suppr.lineNumber;
317+
loc.col = suppr.column;
301318
// cppcheck-suppress useStlAlgorithm
302-
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress Begin: No matching end");
319+
bad.emplace_back(loc, "Suppress Begin: No matching end");
320+
}
303321
}
304322

305323
void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
@@ -312,7 +330,7 @@ void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
312330
::addInlineSuppressions(filedata->tokens, mSettings, suppressions, err);
313331
}
314332
for (const BadInlineSuppression &bad : err) {
315-
error(bad.file, bad.line, bad.errmsg, simplecpp::Output::ERROR); // TODO: use individual (non-fatal) ID
333+
error(bad.location, bad.errmsg, simplecpp::Output::ERROR); // TODO: use individual (non-fatal) ID
316334
}
317335
}
318336

@@ -860,7 +878,7 @@ bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool sh
860878
case simplecpp::Output::ERROR:
861879
hasError = true;
862880
if (!startsWith(out.msg,"#error") || showerror)
863-
error(out.location.file(), out.location.line, out.msg, out.type);
881+
error(out.location, out.msg, out.type);
864882
break;
865883
case simplecpp::Output::WARNING:
866884
case simplecpp::Output::PORTABILITY_BACKSLASH:
@@ -870,20 +888,22 @@ bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool sh
870888
const std::string::size_type pos1 = out.msg.find_first_of("<\"");
871889
const std::string::size_type pos2 = out.msg.find_first_of(">\"", pos1 + 1U);
872890
if (pos1 < pos2 && pos2 != std::string::npos)
873-
missingInclude(out.location.file(), out.location.line, out.location.col, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
891+
missingInclude(out.location, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
874892
}
875893
break;
876894
case simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY:
877895
case simplecpp::Output::SYNTAX_ERROR:
878896
case simplecpp::Output::UNHANDLED_CHAR_ERROR:
879897
hasError = true;
880-
error(out.location.file(), out.location.line, out.msg, out.type);
898+
error(out.location, out.msg, out.type);
881899
break;
882900
case simplecpp::Output::EXPLICIT_INCLUDE_NOT_FOUND:
883901
case simplecpp::Output::FILE_NOT_FOUND:
884902
case simplecpp::Output::DUI_ERROR:
885903
hasError = true;
886-
error("", 0, out.msg, out.type);
904+
std::vector<std::string> f;
905+
simplecpp::Location loc(f);
906+
error(loc, out.msg, out.type);
887907
break;
888908
}
889909
}
@@ -918,15 +938,15 @@ static std::string simplecppErrToId(simplecpp::Output::Type type)
918938
cppcheck::unreachable();
919939
}
920940

921-
void Preprocessor::error(const std::string &filename, unsigned int linenr, const std::string &msg, simplecpp::Output::Type type)
941+
void Preprocessor::error(const simplecpp::Location& loc, const std::string &msg, simplecpp::Output::Type type)
922942
{
923943
std::list<ErrorMessage::FileLocation> locationList;
924-
if (!filename.empty()) {
925-
std::string file = Path::fromNativeSeparators(filename);
944+
if (!loc.file().empty()) {
945+
std::string file = Path::fromNativeSeparators(loc.file());
926946
if (mSettings.relativePaths)
927947
file = Path::getRelativePath(file, mSettings.basePaths);
928948

929-
locationList.emplace_back(file, linenr, 0); // TODO: set column
949+
locationList.emplace_back(file, loc.line, loc.col); // TODO: set column
930950
}
931951
mErrorLogger.reportErr(ErrorMessage(std::move(locationList),
932952
mFile0,
@@ -937,14 +957,14 @@ void Preprocessor::error(const std::string &filename, unsigned int linenr, const
937957
}
938958

939959
// Report that include is missing
940-
void Preprocessor::missingInclude(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &header, HeaderTypes headerType)
960+
void Preprocessor::missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType)
941961
{
942962
if (!mSettings.checks.isEnabled(Checks::missingInclude))
943963
return;
944964

945965
std::list<ErrorMessage::FileLocation> locationList;
946-
if (!filename.empty()) {
947-
locationList.emplace_back(filename, linenr, col);
966+
if (!loc.file().empty()) {
967+
locationList.emplace_back(loc.file(), loc.line, loc.col);
948968
}
949969
ErrorMessage errmsg(std::move(locationList), mFile0, Severity::information,
950970
(headerType==SystemHeader) ?
@@ -960,13 +980,14 @@ void Preprocessor::getErrorMessages(ErrorLogger &errorLogger, const Settings &se
960980
std::vector<std::string> files;
961981
simplecpp::TokenList tokens(files);
962982
Preprocessor preprocessor(tokens, settings, errorLogger, Standards::Language::CPP);
963-
preprocessor.missingInclude("", 1, 2, "", UserHeader);
964-
preprocessor.missingInclude("", 1, 2, "", SystemHeader);
965-
preprocessor.error("", 1, "message", simplecpp::Output::ERROR);
966-
preprocessor.error("", 1, "message", simplecpp::Output::SYNTAX_ERROR);
967-
preprocessor.error("", 1, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
968-
preprocessor.error("", 1, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
969-
preprocessor.error("", 1, "message", simplecpp::Output::FILE_NOT_FOUND);
983+
simplecpp::Location loc(files);
984+
preprocessor.missingInclude(loc, "", UserHeader);
985+
preprocessor.missingInclude(loc, "", SystemHeader);
986+
preprocessor.error(loc, "message", simplecpp::Output::ERROR);
987+
preprocessor.error(loc, "message", simplecpp::Output::SYNTAX_ERROR);
988+
preprocessor.error(loc, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
989+
preprocessor.error(loc, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
990+
preprocessor.error(loc, "message", simplecpp::Output::FILE_NOT_FOUND);
970991
}
971992

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

‎lib/preprocessor.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
142142

143143
bool reportOutput(const simplecpp::OutputList &outputList, bool showerror);
144144

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

147147
private:
148148
static bool hasErrors(const simplecpp::Output &output);
@@ -159,7 +159,7 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
159159
SystemHeader
160160
};
161161

162-
void missingInclude(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &header, HeaderTypes headerType);
162+
void missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType);
163163

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

‎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;
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;

‎test/cli/other_test.py‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3889,8 +3889,7 @@ def test_simplecpp_unhandled_char(tmp_path):
38893889
assert exitcode == 0, stdout
38903890
assert stdout.splitlines() == []
38913891
assert stderr.splitlines() == [
3892-
# TODO: lacks column information
3893-
'{}:2:0: error: The code contains unhandled character(s) (character code=228). Neither unicode nor extended ascii is supported. [unhandledChar]'.format(test_file)
3892+
'{}:2:5: error: The code contains unhandled character(s) (character code=228). Neither unicode nor extended ascii is supported. [unhandledChar]'.format(test_file)
38943893
]
38953894

38963895

@@ -3921,9 +3920,8 @@ def test_simplecpp_include_nested_too_deeply(tmp_path):
39213920
test_h = tmp_path / 'test_398.h'
39223921
assert stderr.splitlines() == [
39233922
# TODO: should only report the error once
3924-
# TODO: lacks column information
3925-
'{}:1:0: error: #include nested too deeply [includeNestedTooDeeply]'.format(test_h),
3926-
'{}:1:0: error: #include nested too deeply [includeNestedTooDeeply]'.format(test_h)
3923+
'{}:1:2: error: #include nested too deeply [includeNestedTooDeeply]'.format(test_h),
3924+
'{}:1:2: error: #include nested too deeply [includeNestedTooDeeply]'.format(test_h)
39273925
]
39283926

39293927

@@ -3944,7 +3942,6 @@ def test_simplecpp_syntax_error(tmp_path):
39443942
assert stdout.splitlines() == []
39453943
assert stderr.splitlines() == [
39463944
# TODO: should only report the error once
3947-
# TODO: lacks column information
3948-
'{}:1:0: error: No header in #include [syntaxError]'.format(test_file),
3949-
'{}:1:0: error: No header in #include [syntaxError]'.format(test_file)
3945+
'{}:1:2: error: No header in #include [syntaxError]'.format(test_file),
3946+
'{}:1:2: error: No header in #include [syntaxError]'.format(test_file)
39503947
]

‎test/testsuppressions.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ class TestSuppressions : public TestFixture {
426426
" a++;\n"
427427
"}\n",
428428
""));
429-
ASSERT_EQUALS("[test.cpp:2:0]: (error) File suppression should be at the top of the file [preprocessorErrorDirective]\n"
429+
ASSERT_EQUALS("[test.cpp:2:5]: (error) File suppression should be at the top of the file [preprocessorErrorDirective]\n"
430430
"[test.cpp:4:5]: (error) Uninitialized variable: a [uninitvar]\n", errout_str());
431431

432432
ASSERT_EQUALS(1, (this->*check)("void f() {\n"
@@ -435,7 +435,7 @@ class TestSuppressions : public TestFixture {
435435
"}\n"
436436
"// cppcheck-suppress-file uninitvar\n",
437437
""));
438-
ASSERT_EQUALS("[test.cpp:5:0]: (error) File suppression should be at the top of the file [preprocessorErrorDirective]\n"
438+
ASSERT_EQUALS("[test.cpp:5:1]: (error) File suppression should be at the top of the file [preprocessorErrorDirective]\n"
439439
"[test.cpp:3:5]: (error) Uninitialized variable: a [uninitvar]\n", errout_str());
440440

441441
ASSERT_EQUALS(0, (this->*check)("// cppcheck-suppress-file uninitvar\n"
@@ -687,7 +687,7 @@ class TestSuppressions : public TestFixture {
687687
" b++;\n"
688688
"}\n",
689689
""));
690-
ASSERT_EQUALS("[test.cpp:2:0]: (error) Suppress Begin: No matching end [preprocessorErrorDirective]\n"
690+
ASSERT_EQUALS("[test.cpp:2:5]: (error) Suppress Begin: No matching end [preprocessorErrorDirective]\n"
691691
"[test.cpp:4:5]: (error) Uninitialized variable: a [uninitvar]\n"
692692
"[test.cpp:6:5]: (error) Uninitialized variable: b [uninitvar]\n", errout_str());
693693

@@ -699,7 +699,7 @@ class TestSuppressions : public TestFixture {
699699
" // cppcheck-suppress-end uninitvar\n"
700700
"}\n",
701701
""));
702-
ASSERT_EQUALS("[test.cpp:6:0]: (error) Suppress End: No matching begin [preprocessorErrorDirective]\n"
702+
ASSERT_EQUALS("[test.cpp:6:5]: (error) Suppress End: No matching begin [preprocessorErrorDirective]\n"
703703
"[test.cpp:3:5]: (error) Uninitialized variable: a [uninitvar]\n"
704704
"[test.cpp:5:5]: (error) Uninitialized variable: b [uninitvar]\n", errout_str());
705705

0 commit comments

Comments
 (0)