kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a subscriber: usaxena95. kadircet requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang.
Normally there are heruistics in lexer to treat `//*` specially in language modes that don't have line comments (to emit `/`). Unfortunately this only applied to the first occurence of a line comment inside the file, as the subsequent line comments were treated as if language had support for them. This unfortunately only holds in normal lexing mode, as in raw mode all occurences of line comments received this treatment, which created discrepancies when comparing expanded and spelled tokens. The proper fix would be to just make sure we treat all the line comments with a subsequent `*` the same way, but it would imply breaking some code that's accepted by clang today. So instead we introduce the same bug into raw lexing mode. Fixes https://github.com/clangd/clangd/issues/1003. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D118471 Files: clang/lib/Lex/Lexer.cpp clang/unittests/Lex/LexerTest.cpp Index: clang/unittests/Lex/LexerTest.cpp =================================================================== --- clang/unittests/Lex/LexerTest.cpp +++ clang/unittests/Lex/LexerTest.cpp @@ -23,6 +23,8 @@ #include "clang/Lex/ModuleLoader.h" #include "clang/Lex/Preprocessor.h" #include "clang/Lex/PreprocessorOptions.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/StringRef.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include <memory> @@ -632,4 +634,27 @@ EXPECT_EQ(SourceMgr.getNumCreatedFIDsForFileID(PP->getPredefinesFileID()), 1U); } + +TEST_F(LexerTest, RawAndNormalLexSameForLineComments) { + const llvm::StringLiteral Source = R"cpp( + // First line comment. + //* Second line comment which is ambigious. + )cpp"; + LangOpts.LineComment = false; + auto Toks = Lex(Source); + auto &SM = PP->getSourceManager(); + auto SrcBuffer = SM.getBufferData(SM.getMainFileID()); + Lexer L(SM.getLocForStartOfFile(SM.getMainFileID()), PP->getLangOpts(), + SrcBuffer.data(), SrcBuffer.data(), + SrcBuffer.data() + SrcBuffer.size()); + + auto ToksView = llvm::makeArrayRef(Toks); + clang::Token T; + while (!L.LexFromRawLexer(T)) { + ASSERT_TRUE(!ToksView.empty()); + EXPECT_EQ(T.getKind(), ToksView.front().getKind()); + ToksView = ToksView.drop_front(); + } + EXPECT_TRUE(ToksView.empty()); +} } // anonymous namespace Index: clang/lib/Lex/Lexer.cpp =================================================================== --- clang/lib/Lex/Lexer.cpp +++ clang/lib/Lex/Lexer.cpp @@ -2378,8 +2378,9 @@ bool &TokAtPhysicalStartOfLine) { // If Line comments aren't explicitly enabled for this language, emit an // extension warning. - if (!LangOpts.LineComment && !isLexingRawMode()) { - Diag(BufferPtr, diag::ext_line_comment); + if (!LangOpts.LineComment) { + if (!isLexingRawMode()) // There's no PP in raw mode, so can't emit diags. + Diag(BufferPtr, diag::ext_line_comment); // Mark them enabled so we only emit one warning for this translation // unit.
Index: clang/unittests/Lex/LexerTest.cpp =================================================================== --- clang/unittests/Lex/LexerTest.cpp +++ clang/unittests/Lex/LexerTest.cpp @@ -23,6 +23,8 @@ #include "clang/Lex/ModuleLoader.h" #include "clang/Lex/Preprocessor.h" #include "clang/Lex/PreprocessorOptions.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/StringRef.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include <memory> @@ -632,4 +634,27 @@ EXPECT_EQ(SourceMgr.getNumCreatedFIDsForFileID(PP->getPredefinesFileID()), 1U); } + +TEST_F(LexerTest, RawAndNormalLexSameForLineComments) { + const llvm::StringLiteral Source = R"cpp( + // First line comment. + //* Second line comment which is ambigious. + )cpp"; + LangOpts.LineComment = false; + auto Toks = Lex(Source); + auto &SM = PP->getSourceManager(); + auto SrcBuffer = SM.getBufferData(SM.getMainFileID()); + Lexer L(SM.getLocForStartOfFile(SM.getMainFileID()), PP->getLangOpts(), + SrcBuffer.data(), SrcBuffer.data(), + SrcBuffer.data() + SrcBuffer.size()); + + auto ToksView = llvm::makeArrayRef(Toks); + clang::Token T; + while (!L.LexFromRawLexer(T)) { + ASSERT_TRUE(!ToksView.empty()); + EXPECT_EQ(T.getKind(), ToksView.front().getKind()); + ToksView = ToksView.drop_front(); + } + EXPECT_TRUE(ToksView.empty()); +} } // anonymous namespace Index: clang/lib/Lex/Lexer.cpp =================================================================== --- clang/lib/Lex/Lexer.cpp +++ clang/lib/Lex/Lexer.cpp @@ -2378,8 +2378,9 @@ bool &TokAtPhysicalStartOfLine) { // If Line comments aren't explicitly enabled for this language, emit an // extension warning. - if (!LangOpts.LineComment && !isLexingRawMode()) { - Diag(BufferPtr, diag::ext_line_comment); + if (!LangOpts.LineComment) { + if (!isLexingRawMode()) // There's no PP in raw mode, so can't emit diags. + Diag(BufferPtr, diag::ext_line_comment); // Mark them enabled so we only emit one warning for this translation // unit.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits