Repository navigation
Fix #714 (# operator escapes backslashes outside string literals) - #715
Conversation
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
There is one regression: if a macro argument ends in an odd number of backslashes outside a literal, the # result is now an unterminated string literal. For example, str(\) now expands to "\" and str(a\) to "a\". That turns puts(str(\)); into puts ( "\" ) ;, which is no longer valid C. The base version gave "\\", which is a valid token. gcc and clang warn "invalid string literal, ignoring final ''" and drop the backslash, so they give "". This is undefined behaviour under C 6.10.3.2p2, but simplecpp should not emit a broken token, because cppcheck tokenizes this output.
Apart from that, the change works. It matches gcc and clang for every other case I tried: plain \ outside literals, "…" and '…' with all prefixes, user-defined literal suffixes like "a\n"_s and 'a'_c, ' inside strings, and " inside character constants. The c99-6_10_3_4_p6.c test from run-tests.py now passes. I found no other problems.
Testing
Built make -j8 testrunner simplecpp CXXOPTS=-Werror in pr/ with no warnings, and built simplecpp in base/ to compare.
./testrunner: exit 0, no failurespython3 run-tests.py: 0 failed (c99-6_10_3_4_p6.c, removed from the todo list, passes)python3 -m pytest integration_test.py: 128 passed./selfcheck.sh: exit 0
Comparison with gcc/clang (gcc -E -P -x c++ -std=c++11):
I passed these arguments to #define str(x) #x:
"a\n"_s, 'a'_c, u8"x\"y", u8'\'', L'\\', a "b\\" c, \\, a\b, '\'', "'", @"x"
The PR output is identical to gcc and clang for all of them. Base escaped ' and backslashes outside literals.
R"(a\b)" gives "\"a\\\\b\"" in both base and pr, while gcc gives "R\"(a\\b)\"". The PR did not change this; it comes from raw strings being converted at tokenization.
Regression: trailing odd backslash
Input file u.c:
#define str(x) #x
puts(str(\));
puts(str(a\));
int x;Command: /work/pr/simplecpp u.c
PR output:
puts ( "\" ) ;
puts ( "a\" ) ;
int x ;
Base output:
puts ( "\\" ) ;
puts ( "a\\" ) ;
int x ;
gcc and clang give "" and "a", with the warning invalid string literal, ignoring final '\'.
Automated review of 8dead66 by Claude. The pull request was built and tested locally.
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
The fix for an odd trailing backslash (str(\)) turns undefined behaviour that gcc and clang only warn about into a fatal error for the whole file. invalidHash is thrown, the expansion fails with syntax_error, and simplecpp gives empty output for the whole translation unit, including all code before the macro use. gcc and clang print warning: invalid string literal, ignoring final '\' and keep going with "". cppcheck would therefore drop the whole file or configuration because of one stringification that the compilers accept. It would be better to drop the last backslash, as gcc does, and maybe report a non-fatal warning.
I found no other new problems. Escaping inside and outside literals matches gcc for prefixes, user-defined literal suffixes, '\'', digit separators, empty arguments and an unused argument that contains str(\). Raw strings (R"(a\b)") and digit separators (1'000 → "1000") still differ from gcc, but base gives the same output, so this PR did not cause them.
Testing
I built make -j8 testrunner simplecpp CXXOPTS=-Werror in pr/ with no warnings, and also built base/.
./testrunner: no failurespython3 run-tests.py: 0 failedpython3 -m pytest integration_test.py: 128 passed./selfcheck.sh: exit 0
The whole file is lost on an odd trailing backslash. c.c:
#define str(x) #x
#if 0
#endif
int before;
const char *s = str(\);
int after;/work/pr/simplecpp c.c prints only blank lines, and then:
c.c:5: syntax error: failed to expand 'str', Invalid # usage when expanding 'str': Stringified argument ends with an odd number of backslashes, which is undefined behavior according to C standard chapter 6.10.3.2, paragraph 2.
Neither int before; nor int after; is output. gcc -E -P c.c warns invalid string literal, ignoring final '\' and outputs:
int before;
const char *s = "";
int after;
str(x \) and str(x\\\) give the same error. gcc turns them into "x " and "x\\".
Other comparisons with gcc -E -P -x c++ -std=c++14. These are identical to gcc: str("a\n"_s), str('\''_c), str(u8"x\"y"), str(a "\\"), str(\ x), str(" \\"), str(), xstr(cat(,)), IGNORE(str(\)) (no error, as with gcc) and F(str(\\)). str(R"(a\b)") and str(1'000) still differ from gcc, but base gives the same output.
Automated review of 8a1f103 by Claude. The pull request was built and tested locally.
| while (trailing < str.size() && str[str.size() - 1U - trailing] == '\\') | ||
| ++trailing; | ||
| if (trailing % 2U == 1U) | ||
| throw invalidHash::unterminatedString(loc, name()); |
There was a problem hiding this comment.
AI review
This throw makes the whole expansion fail. simplecpp then gives empty output for the whole file, including code before this line, and cppcheck loses the translation unit. gcc and clang only warn here (invalid string literal, ignoring final '\') and produce "" for str(\). Repro: int before;\nconst char *s = str(\\);\nint after; gives no output at all, and gcc gives int before; const char *s = ""; int after;. I suggest doing what gcc does: drop the last backslash (str.pop_back()) instead of throwing, and if a diagnostic is wanted, report it as a non-fatal warning.
0dbff5d to
c7fdc70
Compare
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
The trailing-backslash fix now works. str(\) gives "" and reports a non-fatal PORTABILITY_STRINGIFY warning, the same as gcc and clang, and the rest of the file is kept. I found no new correctness problems that block merging.
One minor new case: a \ outside a literal that comes right before a string literal (str(\"a")) now gives the broken token "\\"a\"". The base version gave a valid literal. gcc and clang give the same text, and this is undefined behaviour, so it only matters because cppcheck tokenizes this output. Details are in the line comment.
Also note that PORTABILITY_STRINGIFY is a new Output::Type value, inserted in the middle of the enum. When cppcheck updates simplecpp, its switches over the output types must handle it. If they don't, -Wswitch warns and the message may be dropped silently.
Testing
I built make -j8 testrunner simplecpp CXXOPTS=-Werror in pr/ with no warnings, and built simplecpp in base/ to compare.
./testrunner: exit 0python3 run-tests.py: 0 failedpython3 -m pytest integration_test.py: 128 passed./selfcheck.sh: exit 0
Compared with gcc and clang (gcc -E -P -x c++ -std=c++11, the same for clang). These arguments to #define str(x) #x give the same output as gcc and clang:
"a\n"_s, '\''_c, x\'a', L'\\', \\\, \ \ (same as clang; gcc keeps the final \)
These cases each give exactly one warning, on the right line, the same as gcc:
- nested macros:
T(T(\)),F(str)with#define F(m) m(\),CALL(\)with#define CALL ID(str),ID(str)(\) #if defined(X) || str(\)
An unused argument such as G(str(\)) gives no warning, which is correct. u8R"(a\b)" still differs from gcc, but base gives the same output, so this PR did not cause it.
Backslash right before a string literal. Test file a.cpp:
#define str(x) #x
const char *a = str(\"a");Command: /work/pr/simplecpp a.cpp
- pr:
const char * a = "\\"a\"" ;, a token that is not a valid literal: the string ends after\\anda\""is left over - base:
const char * a = "\\\"a\"" ; - gcc and clang:
"\\"a\"", with no diagnostic
Automated review of c7fdc70 by Claude. The pull request was built and tested locally.
No description provided.