Skip to content

Commit b286e12

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

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
@@ -70,9 +70,8 @@ Preprocessor::Preprocessor(simplecpp::TokenList& tokens, const Settings& setting
7070

7171
namespace {
7272
struct BadInlineSuppression {
73-
BadInlineSuppression(std::string file, const int line, std::string msg) : file(std::move(file)), line(line), errmsg(std::move(msg)) {}
74-
std::string file;
75-
int line;
73+
BadInlineSuppression(const simplecpp::Location& loc, std::string msg) : location(loc), errmsg(std::move(msg)) {}
74+
simplecpp::Location location;
7675
std::string errmsg;
7776
};
7877
}
@@ -134,10 +133,11 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
134133
s.isInline = true;
135134
s.type = errorType;
136135
s.lineNumber = tok->location.line;
136+
s.column = tok->location.col;
137137
}
138138

139139
if (!errmsg.empty())
140-
bad.emplace_back(tok->location.file(), tok->location.line, std::move(errmsg));
140+
bad.emplace_back(tok->location, std::move(errmsg));
141141

142142
std::copy_if(suppressions.cbegin(), suppressions.cend(), std::back_inserter(inlineSuppressions), [](const SuppressionList::Suppression& s) {
143143
return !s.errorId.empty();
@@ -152,12 +152,13 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
152152
s.isInline = true;
153153
s.type = errorType;
154154
s.lineNumber = tok->location.line;
155+
s.column = tok->location.col;
155156

156157
if (!s.errorId.empty())
157158
inlineSuppressions.push_back(std::move(s));
158159

159160
if (!errmsg.empty())
160-
bad.emplace_back(tok->location.file(), tok->location.line, std::move(errmsg));
161+
bad.emplace_back(tok->location, std::move(errmsg));
161162
}
162163

163164
return true;
@@ -232,6 +233,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
232233
// Add the suppressions.
233234
for (SuppressionList::Suppression &suppr : inlineSuppressions) {
234235
suppr.fileName = relativeFilename;
236+
suppr.fileIndex = tok->location.fileIndex;
235237

236238
if (SuppressionList::Type::blockBegin == suppr.type)
237239
{
@@ -254,6 +256,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
254256
suppr.lineBegin = supprBegin->lineNumber;
255257
suppr.lineEnd = suppr.lineNumber;
256258
suppr.lineNumber = supprBegin->lineNumber;
259+
suppr.column = supprBegin->column;
257260
suppr.type = SuppressionList::Type::block;
258261
inlineSuppressionsBlockBegin.erase(supprBegin);
259262
suppressions.addSuppression(std::move(suppr)); // TODO: check result
@@ -265,8 +268,12 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
265268
}
266269

267270
if (throwError) {
271+
simplecpp::Location loc(tokens.getFiles());
268272
// NOLINTNEXTLINE(bugprone-use-after-move) - moved only when thrownError is false
269-
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress End: No matching begin");
273+
loc.fileIndex = suppr.fileIndex;
274+
loc.line = suppr.lineNumber;
275+
loc.col = suppr.column;
276+
bad.emplace_back(loc, "Suppress End: No matching begin");
270277
}
271278
} else if (SuppressionList::Type::unique == suppr.type || suppr.type == SuppressionList::Type::macro) {
272279
// special handling when suppressing { warnings for backwards compatibility
@@ -280,20 +287,31 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
280287

281288
suppr.thisAndNextLine = thisAndNextLine;
282289
suppr.lineNumber = tok->location.line;
290+
suppr.column = tok->location.col;
283291
suppr.macroName = macroName;
284292
suppressions.addSuppression(std::move(suppr)); // TODO: check result
285293
} else if (SuppressionList::Type::file == suppr.type) {
286294
if (onlyComments)
287295
suppressions.addSuppression(std::move(suppr)); // TODO: check result
288-
else
289-
bad.emplace_back(suppr.fileName, suppr.lineNumber, "File suppression should be at the top of the file");
296+
else {
297+
simplecpp::Location loc(tokens.getFiles());
298+
loc.fileIndex = suppr.fileIndex;
299+
loc.line = suppr.lineNumber;
300+
loc.col = suppr.column;
301+
bad.emplace_back(loc, "File suppression should be at the top of the file");
302+
}
290303
}
291304
}
292305
}
293306

294-
for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin)
307+
for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin) {
308+
simplecpp::Location loc(tokens.getFiles());
309+
loc.fileIndex = suppr.fileIndex;
310+
loc.line = suppr.lineNumber;
311+
loc.col = suppr.column;
295312
// cppcheck-suppress useStlAlgorithm
296-
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress Begin: No matching end");
313+
bad.emplace_back(loc, "Suppress Begin: No matching end");
314+
}
297315
}
298316

299317
void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
@@ -306,7 +324,7 @@ void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
306324
::addInlineSuppressions(filedata->tokens, mSettings, suppressions, err);
307325
}
308326
for (const BadInlineSuppression &bad : err) {
309-
error(bad.file, bad.line, bad.errmsg, simplecpp::Output::ERROR); // TODO: use individual (non-fatal) ID
327+
error(bad.location, bad.errmsg, simplecpp::Output::ERROR); // TODO: use individual (non-fatal) ID
310328
}
311329
}
312330

@@ -854,7 +872,7 @@ bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool sh
854872
case simplecpp::Output::ERROR:
855873
hasError = true;
856874
if (!startsWith(out.msg,"#error") || showerror)
857-
error(out.location.file(), out.location.line, out.msg, out.type);
875+
error(out.location, out.msg, out.type);
858876
break;
859877
case simplecpp::Output::WARNING:
860878
case simplecpp::Output::PORTABILITY_BACKSLASH:
@@ -864,20 +882,22 @@ bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool sh
864882
const std::string::size_type pos1 = out.msg.find_first_of("<\"");
865883
const std::string::size_type pos2 = out.msg.find_first_of(">\"", pos1 + 1U);
866884
if (pos1 < pos2 && pos2 != std::string::npos)
867-
missingInclude(out.location.file(), out.location.line, out.location.col, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
885+
missingInclude(out.location, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
868886
}
869887
break;
870888
case simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY:
871889
case simplecpp::Output::SYNTAX_ERROR:
872890
case simplecpp::Output::UNHANDLED_CHAR_ERROR:
873891
hasError = true;
874-
error(out.location.file(), out.location.line, out.msg, out.type);
892+
error(out.location, out.msg, out.type);
875893
break;
876894
case simplecpp::Output::EXPLICIT_INCLUDE_NOT_FOUND:
877895
case simplecpp::Output::FILE_NOT_FOUND:
878896
case simplecpp::Output::DUI_ERROR:
879897
hasError = true;
880-
error("", 0, out.msg, out.type);
898+
std::vector<std::string> f;
899+
simplecpp::Location loc(f);
900+
error(loc, out.msg, out.type);
881901
break;
882902
}
883903
}
@@ -912,15 +932,15 @@ static std::string simplecppErrToId(simplecpp::Output::Type type)
912932
cppcheck::unreachable();
913933
}
914934

915-
void Preprocessor::error(const std::string &filename, unsigned int linenr, const std::string &msg, simplecpp::Output::Type type)
935+
void Preprocessor::error(const simplecpp::Location& loc, const std::string &msg, simplecpp::Output::Type type)
916936
{
917937
std::list<ErrorMessage::FileLocation> locationList;
918-
if (!filename.empty()) {
919-
std::string file = Path::fromNativeSeparators(filename);
938+
if (!loc.file().empty()) {
939+
std::string file = Path::fromNativeSeparators(loc.file());
920940
if (mSettings.relativePaths)
921941
file = Path::getRelativePath(file, mSettings.basePaths);
922942

923-
locationList.emplace_back(file, linenr, 0); // TODO: set column
943+
locationList.emplace_back(file, loc.line, loc.col); // TODO: set column
924944
}
925945
mErrorLogger.reportErr(ErrorMessage(std::move(locationList),
926946
mFile0,
@@ -931,14 +951,14 @@ void Preprocessor::error(const std::string &filename, unsigned int linenr, const
931951
}
932952

933953
// Report that include is missing
934-
void Preprocessor::missingInclude(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &header, HeaderTypes headerType)
954+
void Preprocessor::missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType)
935955
{
936956
if (!mSettings.checks.isEnabled(Checks::missingInclude))
937957
return;
938958

939959
std::list<ErrorMessage::FileLocation> locationList;
940-
if (!filename.empty()) {
941-
locationList.emplace_back(filename, linenr, col);
960+
if (!loc.file().empty()) {
961+
locationList.emplace_back(loc.file(), loc.line, loc.col);
942962
}
943963
ErrorMessage errmsg(std::move(locationList), mFile0, Severity::information,
944964
(headerType==SystemHeader) ?
@@ -954,13 +974,14 @@ void Preprocessor::getErrorMessages(ErrorLogger &errorLogger, const Settings &se
954974
std::vector<std::string> files;
955975
simplecpp::TokenList tokens(files);
956976
Preprocessor preprocessor(tokens, settings, errorLogger, Standards::Language::CPP);
957-
preprocessor.missingInclude("", 1, 2, "", UserHeader);
958-
preprocessor.missingInclude("", 1, 2, "", SystemHeader);
959-
preprocessor.error("", 1, "message", simplecpp::Output::ERROR);
960-
preprocessor.error("", 1, "message", simplecpp::Output::SYNTAX_ERROR);
961-
preprocessor.error("", 1, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
962-
preprocessor.error("", 1, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
963-
preprocessor.error("", 1, "message", simplecpp::Output::FILE_NOT_FOUND);
977+
simplecpp::Location loc(files);
978+
preprocessor.missingInclude(loc, "", UserHeader);
979+
preprocessor.missingInclude(loc, "", SystemHeader);
980+
preprocessor.error(loc, "message", simplecpp::Output::ERROR);
981+
preprocessor.error(loc, "message", simplecpp::Output::SYNTAX_ERROR);
982+
preprocessor.error(loc, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
983+
preprocessor.error(loc, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
984+
preprocessor.error(loc, "message", simplecpp::Output::FILE_NOT_FOUND);
964985
}
965986

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

‎lib/preprocessor.h‎

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

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

144-
void error(const std::string &filename, unsigned int linenr, 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
private:
147147
static bool hasErrors(const simplecpp::Output &output);
@@ -158,7 +158,7 @@ class CPPCHECKLIB WARN_UNUSED Preprocessor {
158158
SystemHeader
159159
};
160160

161-
void missingInclude(const std::string &filename, unsigned int linenr, unsigned int col, const std::string &header, HeaderTypes headerType);
161+
void missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType);
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;
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,9 +3942,8 @@ 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
]
39513948

39523949

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