-
Notifications
You must be signed in to change notification settings - Fork 104
Fix #686: Add warning for bad line directives #687
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 12 commits
40b8f3d
be6e79e
a1240b3
e5ecbd4
d4f1c1f
036b59d
3ee1302
b33c236
d9fbdcd
b9a7613
9642334
40a815f
0cca41f
23ab6e7
d3f8a97
a3c9318
25988a1
f2bd340
2f1fd82
b9309c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -475,26 +475,26 @@ namespace { | |
|
|
||
| simplecpp::TokenList::TokenList(std::vector<std::string> &filenames) : frontToken(nullptr), backToken(nullptr), files(filenames) {} | ||
|
|
||
| simplecpp::TokenList::TokenList(std::istream &istr, std::vector<std::string> &filenames, const std::string &filename, OutputList *outputList) | ||
| simplecpp::TokenList::TokenList(std::istream &istr, std::vector<std::string> &filenames, const std::string &filename, const DUI &dui, OutputList *outputList) | ||
| : frontToken(nullptr), backToken(nullptr), files(filenames) | ||
| { | ||
| StdIStream stream(istr); | ||
| readfile(stream,filename,outputList); | ||
| readfile(stream,filename,dui,outputList); | ||
| } | ||
|
|
||
| simplecpp::TokenList::TokenList(const unsigned char* data, std::size_t size, std::vector<std::string> &filenames, const std::string &filename, OutputList *outputList, int /*unused*/) | ||
| simplecpp::TokenList::TokenList(const unsigned char* data, std::size_t size, std::vector<std::string> &filenames, const std::string &filename, const DUI &dui, OutputList *outputList, int /*unused*/) | ||
| : frontToken(nullptr), backToken(nullptr), files(filenames) | ||
| { | ||
| StdCharBufStream stream(data, size); | ||
| readfile(stream,filename,outputList); | ||
| readfile(stream,filename,dui,outputList); | ||
| } | ||
|
|
||
| simplecpp::TokenList::TokenList(const std::string &filename, std::vector<std::string> &filenames, OutputList *outputList) | ||
| simplecpp::TokenList::TokenList(const std::string &filename, std::vector<std::string> &filenames, const DUI &dui, OutputList *outputList) | ||
| : frontToken(nullptr), backToken(nullptr), files(filenames) | ||
| { | ||
| try { | ||
| FileStream stream(filename, filenames); | ||
| readfile(stream,filename,outputList); | ||
| readfile(stream,filename,dui,outputList); | ||
| } catch (const simplecpp::Output & e) { | ||
| outputList->emplace_back(e); | ||
| } | ||
|
|
@@ -659,13 +659,23 @@ void simplecpp::TokenList::lineDirective(unsigned int fileIndex_, unsigned int l | |
|
|
||
| static const std::string COMMENT_END("*/"); | ||
|
|
||
| void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename, OutputList *outputList) | ||
| void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename, const DUI &dui, OutputList *outputList) | ||
| { | ||
| unsigned int multiline = 0U; | ||
| bool trailing_nl = true; | ||
|
|
||
| const Token *oldLastToken = nullptr; | ||
|
|
||
| const cstd_t cstd = getCStd(dui.std); | ||
| const bool std_is_c = cstd != CUnknown; | ||
| const cppstd_t cppstd = getCppStd(dui.std, std_is_c ? CPPUnknown : CPP26); // use C++26 by default | ||
|
glankk marked this conversation as resolved.
Outdated
|
||
|
|
||
| unsigned long maxline; | ||
|
glankk marked this conversation as resolved.
Outdated
|
||
| if ((cstd != CUnknown && cstd < C99) || (cppstd != CPPUnknown && cppstd < CPP11)) | ||
| maxline = 32767; | ||
| else | ||
| maxline = 2147483647; | ||
|
|
||
| Location location(fileIndex(filename), 1, 1); | ||
| while (stream.good()) { | ||
| unsigned char ch = stream.readChar(); | ||
|
|
@@ -730,7 +740,31 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename, | |
| if (!ppTok || !ppTok->number) | ||
| continue; | ||
|
|
||
| const unsigned int line = std::atol(ppTok->str().c_str()); | ||
| unsigned long line; | ||
| try { | ||
| line = std::stoul(ppTok->str()); | ||
| } catch (...) { | ||
| line = std::numeric_limits<unsigned long>::max(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this value might not work well.. it means tokens might get line number 0 which cppcheck interprets as "no line". I suggest that the value passed in the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. or assign
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually it might be better that we just ignore the line number and use the current one when it's out of range, what do you think?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or just discard the line directive entirely.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be fine with both those options.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've changed it to just use the maximum possible line number, I think that's alright. It's guaranteed to always be representable, and it's also the most lenient option in terms of conditional support. |
||
| } | ||
|
|
||
| if (line == 0 || line > maxline) { | ||
| if (outputList) { | ||
| std::string msg = "Line number out of range: " + ppTok->str() + ". "; | ||
| if (line == 0) { | ||
| msg += "Line number zero is undefined behavior."; | ||
| } else { | ||
| msg += "Line numbers above " + std::to_string(maxline) + " are " + | ||
| (cppstd == CPP26 ? "conditionally supported" : "undefined behavior") + | ||
|
glankk marked this conversation as resolved.
Outdated
|
||
| " in " + (std_is_c ? getCStdName(cstd) : getCppStdName(cppstd)) + "."; | ||
| } | ||
| simplecpp::Output err{ | ||
| simplecpp::Output::PORTABILITY_LINE_DIRECTIVE, | ||
| location, msg | ||
| }; | ||
| outputList->emplace_back(std::move(err)); | ||
| } | ||
| } | ||
|
|
||
| ppTok = advanceAndSkipComments(ppTok); | ||
|
|
||
| unsigned int fileindex; | ||
|
|
@@ -3163,7 +3197,7 @@ std::pair<simplecpp::FileData *, bool> simplecpp::FileDataCache::tryload(FileDat | |
| return {id_it->second, false}; | ||
| } | ||
|
|
||
| auto *const data = new FileData {path, TokenList(path, filenames, outputList)}; | ||
| auto *const data = new FileData {path, TokenList(path, filenames, {}, outputList)}; | ||
|
|
||
| if (dui.removeComments) | ||
| data->tokens.removeComments(); | ||
|
|
@@ -3970,7 +4004,7 @@ void simplecpp::cleanup(FileDataCache &cache) | |
| cache.clear(); | ||
| } | ||
|
|
||
| simplecpp::cstd_t simplecpp::getCStd(const std::string &std) | ||
| simplecpp::cstd_t simplecpp::getCStd(const std::string &std, cstd_t dflt) | ||
|
glankk marked this conversation as resolved.
Outdated
|
||
| { | ||
| if (std == "c90" || std == "c89" || std == "iso9899:1990" || std == "iso9899:199409" || std == "gnu90" || std == "gnu89") | ||
| return C89; | ||
|
|
@@ -3984,7 +4018,21 @@ simplecpp::cstd_t simplecpp::getCStd(const std::string &std) | |
| return C23; | ||
| if (std == "c2y" || std == "gnu2y") | ||
| return C2Y; | ||
| return CUnknown; | ||
| return dflt; | ||
| } | ||
|
|
||
| const char *simplecpp::getCStdName(cstd_t std) | ||
| { | ||
| switch (std) { | ||
| case CUnknown: return "C"; | ||
| case C89: return "C89"; | ||
| case C99: return "C99"; | ||
| case C11: return "C11"; | ||
| case C17: return "C17"; | ||
| case C23: return "C23"; | ||
| case C2Y: return "C2Y"; | ||
| } | ||
| return ""; | ||
| } | ||
|
|
||
| std::string simplecpp::getCStdString(cstd_t std) | ||
|
|
@@ -4020,7 +4068,7 @@ std::string simplecpp::getCStdString(const std::string &std) | |
| return getCStdString(getCStd(std)); | ||
| } | ||
|
|
||
| simplecpp::cppstd_t simplecpp::getCppStd(const std::string &std) | ||
| simplecpp::cppstd_t simplecpp::getCppStd(const std::string &std, cppstd_t dflt) | ||
| { | ||
| if (std == "c++98" || std == "c++03" || std == "gnu++98" || std == "gnu++03") | ||
| return CPP03; | ||
|
|
@@ -4036,7 +4084,22 @@ simplecpp::cppstd_t simplecpp::getCppStd(const std::string &std) | |
| return CPP23; | ||
| if (std == "c++26" || std == "c++2c" || std == "gnu++26" || std == "gnu++2c") | ||
| return CPP26; | ||
| return CPPUnknown; | ||
| return dflt; | ||
| } | ||
|
|
||
| const char *simplecpp::getCppStdName(cppstd_t std) | ||
| { | ||
| switch (std) { | ||
| case CPPUnknown: return "C++"; | ||
| case CPP03: return "C++03"; | ||
| case CPP11: return "C++11"; | ||
| case CPP14: return "C++14"; | ||
| case CPP17: return "C++17"; | ||
| case CPP20: return "C++20"; | ||
| case CPP23: return "C++23"; | ||
| case CPP26: return "C++26"; | ||
| } | ||
| return ""; | ||
| } | ||
|
|
||
| std::string simplecpp::getCppStdString(cppstd_t std) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.