fix #337 - line splicing in comment not handled properly #431
Conversation
|
Here are the corresponding links about this issue: |
|
The PR title is misleading. The fix is to handle line splicing differently. simplecpp doesn't contain any uninitvar checking and there is no uninitialized variable usage I hope. #337 is much more proper. |
clock999
changed the title
simplecpp.cpp
Outdated
| @@ -788,6 +788,35 @@ void simplecpp::TokenList::readfile(Stream &stream, const std::string &filename, | |||
| while (stream.good() && ch != '\r' && ch != '\n') { | |||
There was a problem hiding this comment.
I am thinking if something like this could work:
char lastChar = ' ';
while (stream.good() && (lastChar == '\\' || (ch != '\r' && ch != '\n')) {
currentToken += ch;
lastChar = ch;
ch = stream.readChar();
}
simplecpp.cpp
Outdated
| tmp_ch = stream.readChar(); | ||
| } | ||
| if(!stream.good()) { | ||
| if(!tmp.empty()) { |
There was a problem hiding this comment.
this if (!tmp.empty()) is redundant.
simplecpp.cpp
Outdated
| } | ||
|
|
||
| ch = tmp_ch; | ||
| continue; |
simplecpp.cpp
Outdated
| tmp_ch = stream.readChar(); | ||
| } | ||
| if(!stream.good()) { | ||
| break; |
There was a problem hiding this comment.
this break can be removed right since the outer loop will break if stream is not good.
There was a problem hiding this comment.
Hi Danmar,
I see there is a check in the unit tests:
readfile("//123 \\ \n456", &outputList);
ASSERT_EQUALS("file0,1,portability_backslash,Combination 'backslash space newline' is not portable.\n",
toString(outputList));
Seems that means if there is a space or tab('\t') after a backslash, such a warning needs to be added to the outputList. Do you think that is necessary? Sorry for troubling you with this question. I don't know why we need to do that. I updated the commit, but just removed the above test case.
There was a problem hiding this comment.
I do have a feeling that it's a portability issue that we would like to warn about. I have the feeling that the standard only talks about <backslash><newline> not <backslash><whitespace><newline>. If you can see that the standard define the behavior for <backslash><whitespace><newline> then a warning is not needed.
There was a problem hiding this comment.
C99 standard, 5.1.1.2 Translation phases says:
Each instance of a backslash character () immediately followed by a new-line
character is deleted, splicing physical source lines to form logical source lines
That "immediately" indicates to me that it's not standard behavior to ignore whitespace.
danmar
changed the title
To fix the issue:
#337