Skip to content

Commit 7627635

Browse files
firewavefirewave
committed
fixed error location for simplecpp::Output errors [skip ci]
1 parent 825f9fc commit 7627635

File tree

4 files changed

+80
-37
lines changed

4 files changed

+80
-37
lines changed

‎lib/preprocessor.cpp‎

Lines changed: 70 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
}
@@ -143,7 +142,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
143142
}
144143

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

148147
std::copy_if(suppressions.cbegin(), suppressions.cend(), std::back_inserter(inlineSuppressions), [](const SuppressionList::Suppression& s) {
149148
return !s.errorId.empty();
@@ -163,7 +162,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
163162
inlineSuppressions.push_back(std::move(s));
164163

165164
if (!errmsg.empty())
166-
bad.emplace_back(tok->location.file(), tok->location.line, std::move(errmsg));
165+
bad.emplace_back(tok->location, std::move(errmsg));
167166
}
168167

169168
return true;
@@ -238,6 +237,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
238237
// Add the suppressions.
239238
for (SuppressionList::Suppression &suppr : inlineSuppressions) {
240239
suppr.fileName = relativeFilename;
240+
suppr.fileIndex = tok->location.fileIndex;
241241

242242
if (SuppressionList::Type::blockBegin == suppr.type)
243243
{
@@ -271,8 +271,12 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
271271
}
272272

273273
if (throwError) {
274+
simplecpp::Location loc(tokens.getFiles());
274275
// NOLINTNEXTLINE(bugprone-use-after-move) - moved only when thrownError is false
275-
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress End: No matching begin");
276+
loc.fileIndex = suppr.fileIndex;
277+
loc.line = suppr.lineNumber;
278+
loc.col = suppr.column;
279+
bad.emplace_back(loc, "Suppress End: No matching begin");
276280
}
277281
} else if (SuppressionList::Type::unique == suppr.type || suppr.type == SuppressionList::Type::macro) {
278282
// special handling when suppressing { warnings for backwards compatibility
@@ -291,15 +295,25 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
291295
} else if (SuppressionList::Type::file == suppr.type) {
292296
if (onlyComments)
293297
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");
298+
else {
299+
simplecpp::Location loc(tokens.getFiles());
300+
loc.fileIndex = suppr.fileIndex;
301+
loc.line = suppr.lineNumber;
302+
loc.col = suppr.column;
303+
bad.emplace_back(loc, "File suppression should be at the top of the file");
304+
}
296305
}
297306
}
298307
}
299308

300-
for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin)
309+
for (const SuppressionList::Suppression & suppr: inlineSuppressionsBlockBegin) {
310+
simplecpp::Location loc(tokens.getFiles());
311+
loc.fileIndex = suppr.fileIndex;
312+
loc.line = suppr.lineNumber;
313+
loc.col = suppr.column;
301314
// cppcheck-suppress useStlAlgorithm
302-
bad.emplace_back(suppr.fileName, suppr.lineNumber, "Suppress Begin: No matching end");
315+
bad.emplace_back(loc, "Suppress Begin: No matching end");
316+
}
303317
}
304318

305319
void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
@@ -312,7 +326,7 @@ void Preprocessor::inlineSuppressions(SuppressionList &suppressions)
312326
::addInlineSuppressions(filedata->tokens, mSettings, suppressions, err);
313327
}
314328
for (const BadInlineSuppression &bad : err) {
315-
error(bad.file, bad.line, bad.errmsg);
329+
error(bad.location, bad.errmsg, simplecpp::Output::ERROR);
316330
}
317331
}
318332

@@ -860,7 +874,7 @@ bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool sh
860874
case simplecpp::Output::ERROR:
861875
hasError = true;
862876
if (!startsWith(out.msg,"#error") || showerror)
863-
error(out.location.file(), out.location.line, out.msg);
877+
error(out.location, out.msg, out.type);
864878
break;
865879
case simplecpp::Output::WARNING:
866880
case simplecpp::Output::PORTABILITY_BACKSLASH:
@@ -869,54 +883,80 @@ bool Preprocessor::reportOutput(const simplecpp::OutputList &outputList, bool sh
869883
const std::string::size_type pos1 = out.msg.find_first_of("<\"");
870884
const std::string::size_type pos2 = out.msg.find_first_of(">\"", pos1 + 1U);
871885
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);
886+
missingInclude(out.location, out.msg.substr(pos1+1, pos2-pos1-1), out.msg[pos1] == '\"' ? UserHeader : SystemHeader);
873887
}
874888
break;
875889
case simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY:
876890
case simplecpp::Output::SYNTAX_ERROR:
877891
case simplecpp::Output::UNHANDLED_CHAR_ERROR:
878892
hasError = true;
879-
error(out.location.file(), out.location.line, out.msg);
893+
error(out.location, out.msg, out.type);
880894
break;
881895
case simplecpp::Output::EXPLICIT_INCLUDE_NOT_FOUND:
882896
case simplecpp::Output::FILE_NOT_FOUND:
883897
case simplecpp::Output::DUI_ERROR:
884898
hasError = true;
885-
error("", 0, out.msg);
899+
std::vector<std::string> f;
900+
simplecpp::Location loc(f);
901+
error(loc, out.msg, out.type);
886902
break;
887903
}
888904
}
889905

890906
return hasError;
891907
}
892908

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

901-
locationList.emplace_back(file, linenr, 0);
941+
locationList.emplace_back(file, loc.line, loc.col);
902942
}
903943
mErrorLogger.reportErr(ErrorMessage(std::move(locationList),
904944
mFile0,
905945
Severity::error,
906946
msg,
907-
"preprocessorErrorDirective",
947+
simplecppErrToId(type),
908948
Certainty::normal));
909949
}
910950

911951
// Report that include is missing
912-
void Preprocessor::missingInclude(const std::string &filename, unsigned int linenr, const std::string &header, HeaderTypes headerType)
952+
void Preprocessor::missingInclude(const simplecpp::Location& loc, const std::string &header, HeaderTypes headerType)
913953
{
914954
if (!mSettings.checks.isEnabled(Checks::missingInclude))
915955
return;
916956

917957
std::list<ErrorMessage::FileLocation> locationList;
918-
if (!filename.empty()) {
919-
locationList.emplace_back(filename, linenr, 0);
958+
if (!loc.file().empty()) {
959+
locationList.emplace_back(loc.file(), loc.line, loc.col);
920960
}
921961
ErrorMessage errmsg(std::move(locationList), mFile0, Severity::information,
922962
(headerType==SystemHeader) ?
@@ -932,9 +972,13 @@ void Preprocessor::getErrorMessages(ErrorLogger &errorLogger, const Settings &se
932972
std::vector<std::string> files;
933973
simplecpp::TokenList tokens(files);
934974
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 ..
975+
simplecpp::Location loc(files);
976+
preprocessor.missingInclude(loc, "", UserHeader);
977+
preprocessor.missingInclude(loc, "", SystemHeader);
978+
preprocessor.error(loc, "message", simplecpp::Output::ERROR);
979+
preprocessor.error(loc, "message", simplecpp::Output::SYNTAX_ERROR);
980+
preprocessor.error(loc, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR);
981+
preprocessor.error(loc, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY);
938982
}
939983

940984
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
]

0 commit comments

Comments
 (0)