Fix #686: Add warning for bad line directives#687
Conversation
Co-authored-by: Daniel Marjamäki <daniel.marjamaki@gmail.com>
Co-authored-by: Daniel Marjamäki <daniel.marjamaki@gmail.com>
| try { | ||
| line = std::stoul(ppTok->str()); | ||
| } catch (...) { | ||
| line = std::numeric_limits<unsigned long>::max(); |
There was a problem hiding this comment.
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 lineDirective call will be 1. After reporting the error message.. change it to 1?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Or just discard the line directive entirely.
| ";\n"; | ||
| simplecpp::OutputList outputList; | ||
| makeTokenList(code, dui, &outputList); | ||
| ASSERT_EQUALS("file0,2,portability_line_directive,Line number out of range: 18446744073709551617. Line numbers above 2147483647 are conditionally supported in " + std_name + ".\n", |
There was a problem hiding this comment.
In theory this test will behave differently on a platform where unsigned long is 32 bits. I.e. on WIN32.
I don't have a strong opinion.. but it's a bit unfortunate.
There was a problem hiding this comment.
std::stoul which is used for parsing line numbers does not overflow but throws an out of range exception. So this will behave the same everywhere unsigned long is at least 32 bits.
There was a problem hiding this comment.
I assume that's what you're referring to, if you mean the line number in the warning then it's just the token string.
2 participants
readfilenow takes aDUIwith information about which standard to use, which is needed to determine if a warning should be issued or not.preprocessnow also includes outputs frommakeTokenListswhich callsreadfile. Some tests had to be fixed to have an ending newline because of this in order to pass.