dexonsmith requested changes to this revision.
dexonsmith added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang/lib/Lex/Lexer.cpp:2014-2015
+ // getAndAdvanceChar.
+ if (C == '\\')
+ C = getAndAdvanceChar(CurPtr, Result);
+
----------------
If `CurPtr` is already equal to `BufferEnd`, why is it safe to call
`getAndAdvanceChar`? Is `BufferEnd` dereferenceable?
================
Comment at: clang/lib/Lex/Lexer.cpp:2026
+
+ if (C == 0) {
NulCharacter = CurPtr-1;
----------------
Should this check still be skipped (in an `else if` of the `C == '\\'` check)?
================
Comment at: clang/unittests/Lex/LexerTest.cpp:477
TEST_F(LexerTest, AvoidPastEndOfStringDereference) {
- std::vector<Token> LexedTokens = Lex(" // \\\n");
- EXPECT_TRUE(LexedTokens.empty());
+ EXPECT_TRUE(Lex(" // \\\n").empty());
+ // rdar://problem/35572754
----------------
To minimize the diff, please separate this change into an NFC commit ahead of
time.
================
Comment at: clang/unittests/Lex/LexerTest.cpp:478
+ EXPECT_TRUE(Lex(" // \\\n").empty());
+ // rdar://problem/35572754
+ EXPECT_TRUE(Lex("#include <\\\\").empty());
----------------
Usually we don't put rdar/bug numbers in the source file. It would make sense
in the commit message though.
https://reviews.llvm.org/D41423
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
