lichray marked 4 inline comments as done. lichray added inline comments.
================ Comment at: clang/include/clang/Basic/CharInfo.h:199-200 + return "\\t"; + case '\v': + return "\\v"; + } ---------------- aaron.ballman wrote: > lichray wrote: > > aaron.ballman wrote: > > > We're also missing `\?` right? > > `?` does not seem to need `"escaping?"` > It's the only simple escape sequence we're not handling here: > http://eel.is/c++draft/lex.literal#nt:simple-escape-sequence-char > > (You need to escape `?` thanks to trigraphs. Consider a string literal like > `"This does what now??!"`.) > It's the only simple escape sequence we're not handling here: > http://eel.is/c++draft/lex.literal#nt:simple-escape-sequence-char > > (You need to escape `?` thanks to trigraphs. Consider a string literal like > `"This does what now??!"`.) Hmm, I think we're safe here ``` ~/devel/llvm-project> ./build/bin/clang++ -fsyntax-only -std=c++14 c.cc c.cc:1:50: warning: trigraph converted to '|' character [-Wtrigraphs] void foo() { char const s[] = "This does what now??!"; } ^ 1 warning generated. ~/devel/llvm-project> ./build/bin/clang++ -fsyntax-only -std=c++20 c.cc c.cc:1:50: warning: trigraph ignored [-Wtrigraphs] void foo() { char const s[] = "This does what now??!"; } ^ 1 warning generated. ``` For these reasons, 1. I don't want users to copy diagnosis messages, paste them into a program, and get something different. This has been prevented by the warnings. 2. We support trigraph up to C++14, way behind the version (C++20) we can use string literals in NTTP. So it should be mostly fine. 3. In C++14 we can use the characters in NTTP, but that one has a different diagnosis: ``` template <char C> class ASCII {}; void Foo(ASCII<'?'>); void Foo(ASCII<'??!'>); void test_pascal() { ASCII<'!'> a; Foo(a); } ``` ``` c.cc:4:17: warning: trigraph converted to '|' character [-Wtrigraphs] void Foo(ASCII<'??!'>); ^ c.cc:7:3: error: no matching function for call to 'Foo' Foo(a); ^~~ c.cc:3:6: note: candidate function not viable: no known conversion from 'ASCII<'!' aka 33>' to 'ASCII<'?' aka 63>' for 1st argument void Foo(ASCII<'?'>); ^ c.cc:4:6: note: candidate function not viable: no known conversion from 'ASCII<'!' aka 33>' to 'ASCII<'|' aka 124>' for 1st argument void Foo(ASCII<'??!'>); ^ 1 warning and 1 error generated. ``` Looks nice to me even if we don't escape. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115031/new/ https://reviews.llvm.org/D115031 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits