[clang-tools-extra] r359035 - Re-apply r357823 "[Lexer] NFC: Fix an off-by-one bug in getAsCharRange()."
Author: dergachev Date: Tue Apr 23 14:15:26 2019 New Revision: 359035 URL: http://llvm.org/viewvc/llvm-project?rev=359035=rev Log: Re-apply r357823 "[Lexer] NFC: Fix an off-by-one bug in getAsCharRange()." It now comes with a follow-up fix for the clients of this API in clangd and clang-tidy. Differential Revision: https://reviews.llvm.org/D59977 Modified: clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp Modified: clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp?rev=359035=359034=359035=diff == --- clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp Tue Apr 23 14:15:26 2019 @@ -39,6 +39,7 @@ static bool alreadyConcatenated(std::siz const SourceRange , const SourceManager , const LangOptions ) { + // FIXME: This logic breaks when there is a comment with ':'s in the middle. CharSourceRange TextRange = Lexer::getAsCharRange(ReplacementRange, Sources, LangOpts); StringRef CurrentNamespacesText = Modified: clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp?rev=359035=359034=359035=diff == --- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp Tue Apr 23 14:15:26 2019 @@ -102,11 +102,14 @@ void NamespaceCommentCheck::check(const } } + // FIXME: This probably breaks on comments between the namespace and its '{'. auto TextRange = Lexer::getAsCharRange(SourceRange(NestedNamespaceBegin, LBracketLocation), Sources, getLangOpts()); StringRef NestedNamespaceName = - Lexer::getSourceText(TextRange, Sources, getLangOpts()).rtrim(); + Lexer::getSourceText(TextRange, Sources, getLangOpts()) + .rtrim('{') // Drop the { itself. + .rtrim(); // Drop any whitespace before it. bool IsNested = NestedNamespaceName.contains(':'); if (IsNested) Modified: clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp?rev=359035=359034=359035=diff == --- clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/SelectionTests.cpp Tue Apr 23 14:15:26 2019 @@ -45,7 +45,7 @@ Range nodeRange(const SelectionTree::Nod CharSourceRange R = Lexer::getAsCharRange(SR, SM, AST.getASTContext().getLangOpts()); return Range{offsetToPosition(Buffer, SM.getFileOffset(R.getBegin())), - offsetToPosition(Buffer, SM.getFileOffset(R.getEnd()) + 1)}; + offsetToPosition(Buffer, SM.getFileOffset(R.getEnd()))}; } std::string nodeKind(const SelectionTree::Node *N) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r359035 - Re-apply r357823 "[Lexer] NFC: Fix an off-by-one bug in getAsCharRange()."
Author: dergachev Date: Tue Apr 23 14:15:26 2019 New Revision: 359035 URL: http://llvm.org/viewvc/llvm-project?rev=359035=rev Log: Re-apply r357823 "[Lexer] NFC: Fix an off-by-one bug in getAsCharRange()." It now comes with a follow-up fix for the clients of this API in clangd and clang-tidy. Differential Revision: https://reviews.llvm.org/D59977 Modified: cfe/trunk/include/clang/Basic/PlistSupport.h cfe/trunk/include/clang/Lex/Lexer.h cfe/trunk/unittests/Lex/LexerTest.cpp Modified: cfe/trunk/include/clang/Basic/PlistSupport.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/PlistSupport.h?rev=359035=359034=359035=diff == --- cfe/trunk/include/clang/Basic/PlistSupport.h (original) +++ cfe/trunk/include/clang/Basic/PlistSupport.h Tue Apr 23 14:15:26 2019 @@ -127,7 +127,11 @@ inline void EmitRange(raw_ostream , co assert(R.isCharRange() && "cannot handle a token range"); Indent(o, indent) << "\n"; EmitLocation(o, SM, R.getBegin(), FM, indent + 1); - EmitLocation(o, SM, R.getEnd(), FM, indent + 1); + + // The ".getLocWithOffset(-1)" emulates the behavior of an off-by-one bug + // in Lexer that is already fixed. It is here for backwards compatibility + // even though it is incorrect. + EmitLocation(o, SM, R.getEnd().getLocWithOffset(-1), FM, indent + 1); Indent(o, indent) << "\n"; } Modified: cfe/trunk/include/clang/Lex/Lexer.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Lexer.h?rev=359035=359034=359035=diff == --- cfe/trunk/include/clang/Lex/Lexer.h (original) +++ cfe/trunk/include/clang/Lex/Lexer.h Tue Apr 23 14:15:26 2019 @@ -382,7 +382,7 @@ public: SourceLocation End = getLocForEndOfToken(Range.getEnd(), 0, SM, LangOpts); return End.isInvalid() ? CharSourceRange() : CharSourceRange::getCharRange( - Range.getBegin(), End.getLocWithOffset(-1)); + Range.getBegin(), End); } static CharSourceRange getAsCharRange(CharSourceRange Range, const SourceManager , Modified: cfe/trunk/unittests/Lex/LexerTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Lex/LexerTest.cpp?rev=359035=359034=359035=diff == --- cfe/trunk/unittests/Lex/LexerTest.cpp (original) +++ cfe/trunk/unittests/Lex/LexerTest.cpp Tue Apr 23 14:15:26 2019 @@ -513,4 +513,23 @@ TEST_F(LexerTest, StringizingRasString) EXPECT_EQ(String6, R"(a\\\n\n\nb)"); } +TEST_F(LexerTest, CharRangeOffByOne) { + std::vector toks = Lex(R"(#define MOO 1 +void foo() { MOO; })"); + const Token = toks[5]; + + EXPECT_EQ(getSourceText(moo, moo), "MOO"); + + SourceRange R{moo.getLocation(), moo.getLocation()}; + + EXPECT_TRUE( + Lexer::isAtStartOfMacroExpansion(R.getBegin(), SourceMgr, LangOpts)); + EXPECT_TRUE( + Lexer::isAtEndOfMacroExpansion(R.getEnd(), SourceMgr, LangOpts)); + + CharSourceRange CR = Lexer::getAsCharRange(R, SourceMgr, LangOpts); + + EXPECT_EQ(Lexer::getSourceText(CR, SourceMgr, LangOpts), "MOO"); // Was "MO". +} + } // anonymous namespace ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits