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

2019-04-23 Thread Artem Dergachev via cfe-commits
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()."

2019-04-23 Thread Artem Dergachev via cfe-commits
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