Skip to content

Commit 0de719a

Browse files
firewavefirewave
committed
fixed #14265 - fixed error location for several preprocessor errors [skip ci]
1 parent 825f9fc commit 0de719a

File tree

6 files changed

+89
-42
lines changed

6 files changed

+89
-42
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: 74 additions & 26 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);
333+
error(bad.location, bad.errmsg, simplecpp::Output::ERROR);
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);
881+
error(out.location, out.msg, out.type);
864882
break;
865883
case simplecpp::Output::WARNING:
866884
case simplecpp::Output::PORTABILITY_BACKSLASH:
@@ -869,54 +887,80 @@ bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool sh
869887
const std::string::size_type pos1 = out.msg.find_first_of("<\"");
870888
const std::string::size_type pos2 = out.msg.find_first_of(">\"", pos1 + 1U);
871889
if (pos1 < pos2 && pos2 != std::string::npos)
872-
missingInclude(out.location.file(), out.location.line, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
890+
missingInclude(out.location, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
873891
}
874892
break;
875893
case simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY:
876894
case simplecpp::Output::SYNTAX_ERROR:
877895
case simplecpp::Output::UNHANDLED_CHAR_ERROR:
878896
hasError = true;
879-
error(out.location.file(), out.location.line, out.msg);
897+
error(out.location, out.msg, out.type);
880898
break;
881899
case simplecpp::Output::EXPLICIT_INCLUDE_NOT_FOUND:
882900
case simplecpp::Output::FILE_NOT_FOUND:
883901
case simplecpp::Output::DUI_ERROR:
884902
hasError = true;
885-
error("", 0, out.msg);
903+
std::vector<std::string> f;
904+
simplecpp::Location loc(f);
905+
error(loc, out.msg, out.type);
886906
break;
887907
}
888908
}
889909

890910
return hasError;
891911
}
892912

893-
void Preprocessor::error(const std::string &filename, unsigned int linenr, const std::string &msg)
913+
static std::string simplecppErrToId(simplecpp::Output::Type type)
914+
{
915+
switch (type) {
916+
case simplecpp::Output::ERROR:
917+
return "preprocessorErrorDirective";
918+
case simplecpp::Output::SYNTAX_ERROR:
919+
return "syntaxError";
920+
case simplecpp::Output::UNHANDLED_CHAR_ERROR:
921+
return "unhandledChar";
922+
case simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY:
923+
return "includeNestedTooDeeply";
924+
// should never occur
925+
case simplecpp::Output::EXPLICIT_INCLUDE_NOT_FOUND:
926+
case simplecpp::Output::FILE_NOT_FOUND:
927+
case simplecpp::Output::DUI_ERROR:
928+
// handled separately
929+
case simplecpp::Output::MISSING_HEADER:
930+
// no handled at all (warnings)
931+
case simplecpp::Output::WARNING:
932+
case simplecpp::Output::PORTABILITY_BACKSLASH:
933+
throw std::runtime_error("unexpected type");
934+
}
935+
}
936+
937+
void Preprocessor::error(const simplecpp::Location& loc, const std::string &msg, simplecpp::Output::Type type)
894938
{
895939
std::list<ErrorMessage::FileLocation> locationList;
896-
if (!filename.empty()) {
897-
std::string file = Path::fromNativeSeparators(filename);
940+
if (!loc.file().empty()) {
941+
std::string file = Path::fromNativeSeparators(loc.file());
898942
if (mSettings.relativePaths)
899943
file = Path::getRelativePath(file, mSettings.basePaths);
900944

901-
locationList.emplace_back(file, linenr, 0);
945+
locationList.emplace_back(file, loc.line, loc.col);
902946
}
903947
mErrorLogger.reportErr(ErrorMessage(std::move(locationList),
904948
mFile0,
905949
Severity::error,
906950
msg,
907-
"preprocessorErrorDirective",
951+
simplecppErrToId(type),
908952
Certainty::normal));
909953
}
910954

911955
// Report that include is missing
912-
void Preprocessor::missingInclude(const std::string &filename, unsigned int linenr, const std::string &header, HeaderTypes headerType)
956+
void Preprocessor::missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType)
913957
{
914958
if (!mSettings.checks.isEnabled(Checks::missingInclude))
915959
return;
916960

917961
std::list<ErrorMessage::FileLocation> locationList;
918-
if (!filename.empty()) {
919-
locationList.emplace_back(filename, linenr, 0);
962+
if (!loc.file().empty()) {
963+
locationList.emplace_back(loc.file(), loc.line, loc.col);
920964
}
921965
ErrorMessage errmsg(std::move(locationList), mFile0, Severity::information,
922966
(headerType==SystemHeader) ?
@@ -932,9 +976,13 @@ void Preprocessor::getErrorMessages(ErrorLogger &errorLogger, const Settings &se
932976
std::vector<std::string> files;
933977
simplecpp::TokenList tokens(files);
934978
Preprocessor preprocessor(tokens, settings, errorLogger, Standards::Language::CPP);
935-
preprocessor.missingInclude("", 1, "", UserHeader);
936-
preprocessor.missingInclude("", 1, "", SystemHeader);
937-
preprocessor.error("", 1, "#error message"); // #error ..
979+
simplecpp::Location loc(files);
980+
preprocessor.missingInclude(loc, "", UserHeader);
981+
preprocessor.missingInclude(loc, "", SystemHeader);
982+
preprocessor.error(loc, "message", simplecpp::Output::ERROR);
983+
preprocessor.error(loc, "message", simplecpp::Output::SYNTAX_ERROR);
984+
preprocessor.error(loc, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
985+
preprocessor.error(loc, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
938986
}
939987

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

‎lib/preprocessor.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,8 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
157157
SystemHeader
158158
};
159159

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

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

@@ -172,7 +172,7 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
172172
simplecpp::FileDataCache mFileCache;
173173

174174
/** filename for cpp/c file - useful when reporting errors */
175-
std::string mFile0;
175+
std::string mFile0; // TODO: this is never set
176176
Standards::Language mLang{Standards::Language::None};
177177

178178
/** simplecpp tracking info */

‎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: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def test_missing_include(tmpdir): # #11283
3131
args = ['--enable=missingInclude', '--template=simple', test_file]
3232

3333
_, _, stderr = cppcheck(args)
34-
assert stderr == '{}:2:0: information: Include file: "test.h" not found. [missingInclude]\n'.format(test_file)
34+
assert stderr == '{}:2:18: information: Include file: "test.h" not found. [missingInclude]\n'.format(test_file)
3535

3636

3737
def __test_missing_include_check_config(tmpdir, use_j):
@@ -3893,9 +3893,7 @@ def test_simplecpp_unhandled_char(tmp_path):
38933893
assert exitcode == 0, stdout
38943894
assert stdout.splitlines() == []
38953895
assert stderr.splitlines() == [
3896-
# TODO: lacks column information
3897-
# TODO: should report another ID
3898-
'{}:2:0: error: The code contains unhandled character(s) (character code=228). Neither unicode nor extended ascii is supported. [preprocessorErrorDirective]'.format(test_file)
3896+
'{}:2:5: error: The code contains unhandled character(s) (character code=228). Neither unicode nor extended ascii is supported. [unhandledChar]'.format(test_file)
38993897
]
39003898

39013899

@@ -3925,8 +3923,7 @@ def test_simplecpp_include_nested_too_deeply(tmp_path):
39253923
test_h = tmp_path / 'test_398.h'
39263924
assert stderr.splitlines() == [
39273925
# TODO: should only report the error once
3928-
# TODO: should report another ID
3929-
'{}:1:0: error: #include nested too deeply [preprocessorErrorDirective]'.format(test_h),
3926+
'{}:1:2: error: #include nested too deeply [includeNestedTooDeeply]'.format(test_h),
39303927
'{}:1:2: error: #include nested too deeply [preprocessorErrorDirective]'.format(test_h)
39313928
]
39323929

@@ -3947,7 +3944,6 @@ def test_simplecpp_syntax_error(tmp_path):
39473944
assert stdout.splitlines() == []
39483945
assert stderr.splitlines() == [
39493946
# TODO: should only report the error once
3950-
# TODO: should report another ID
3951-
'{}:1:0: error: No header in #include [preprocessorErrorDirective]'.format(test_file),
3947+
'{}:1:2: error: No header in #include [syntaxError]'.format(test_file),
39523948
'{}:1:2: error: No header in #include [preprocessorErrorDirective]'.format(test_file)
39533949
]

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