[clang-tools-extra] r359035 - Re-apply r357823 "[Lexer] NFC: Fix an off-by-one bug in getAsCharRange()."

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 23 14:15:26 PDT 2019


Author: dergachev
Date: Tue Apr 23 14:15:26 2019
New Revision: 359035

URL: http://llvm.org/viewvc/llvm-project?rev=359035&view=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&r1=359034&r2=359035&view=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 &ReplacementRange,
                                 const SourceManager &Sources,
                                 const LangOptions &LangOpts) {
+  // 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&r1=359034&r2=359035&view=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&r1=359034&r2=359035&view=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) {




More information about the cfe-commits mailing list