r238922 - Allow replacements created from token ranges to specify language options.

Manuel Klimek klimek at google.com
Wed Jun 3 06:10:41 PDT 2015


Author: klimek
Date: Wed Jun  3 08:10:41 2015
New Revision: 238922

URL: http://llvm.org/viewvc/llvm-project?rev=238922&view=rev
Log:
Allow replacements created from token ranges to specify language options.

The default language options will lead to incorrect replacements in C++
code, for example when trying to replace nested name specifiers ending
in "::".

Modified:
    cfe/trunk/include/clang/Tooling/Core/Replacement.h
    cfe/trunk/lib/Tooling/Core/Replacement.cpp
    cfe/trunk/unittests/Tooling/RefactoringTest.cpp

Modified: cfe/trunk/include/clang/Tooling/Core/Replacement.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Core/Replacement.h?rev=238922&r1=238921&r2=238922&view=diff
==============================================================================
--- cfe/trunk/include/clang/Tooling/Core/Replacement.h (original)
+++ cfe/trunk/include/clang/Tooling/Core/Replacement.h Wed Jun  3 08:10:41 2015
@@ -19,6 +19,7 @@
 #ifndef LLVM_CLANG_TOOLING_CORE_REPLACEMENT_H
 #define LLVM_CLANG_TOOLING_CORE_REPLACEMENT_H
 
+#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/StringRef.h"
 #include <set>
@@ -77,22 +78,24 @@ public:
   /// \param FilePath A source file accessible via a SourceManager.
   /// \param Offset The byte offset of the start of the range in the file.
   /// \param Length The length of the range in bytes.
-  Replacement(StringRef FilePath, unsigned Offset,
-              unsigned Length, StringRef ReplacementText);
+  Replacement(StringRef FilePath, unsigned Offset, unsigned Length,
+              StringRef ReplacementText);
 
   /// \brief Creates a Replacement of the range [Start, Start+Length) with
   /// ReplacementText.
-  Replacement(const SourceManager &Sources, SourceLocation Start, unsigned Length,
-              StringRef ReplacementText);
+  Replacement(const SourceManager &Sources, SourceLocation Start,
+              unsigned Length, StringRef ReplacementText);
 
   /// \brief Creates a Replacement of the given range with ReplacementText.
   Replacement(const SourceManager &Sources, const CharSourceRange &Range,
-              StringRef ReplacementText);
+              StringRef ReplacementText,
+              const LangOptions &LangOpts = LangOptions());
 
   /// \brief Creates a Replacement of the node with ReplacementText.
   template <typename Node>
   Replacement(const SourceManager &Sources, const Node &NodeToReplace,
-              StringRef ReplacementText);
+              StringRef ReplacementText,
+              const LangOptions &LangOpts = LangOptions());
 
   /// \brief Returns whether this replacement can be applied to a file.
   ///
@@ -114,11 +117,13 @@ public:
   std::string toString() const;
 
  private:
-  void setFromSourceLocation(const SourceManager &Sources, SourceLocation Start,
-                             unsigned Length, StringRef ReplacementText);
-  void setFromSourceRange(const SourceManager &Sources,
-                          const CharSourceRange &Range,
-                          StringRef ReplacementText);
+   void setFromSourceLocation(const SourceManager &Sources,
+                              SourceLocation Start, unsigned Length,
+                              StringRef ReplacementText);
+   void setFromSourceRange(const SourceManager &Sources,
+                           const CharSourceRange &Range,
+                           StringRef ReplacementText,
+                           const LangOptions &LangOpts);
 
   std::string FilePath;
   Range ReplacementRange;
@@ -217,10 +222,11 @@ std::string applyAllReplacements(StringR
 
 template <typename Node>
 Replacement::Replacement(const SourceManager &Sources,
-                         const Node &NodeToReplace, StringRef ReplacementText) {
+                         const Node &NodeToReplace, StringRef ReplacementText,
+                         const LangOptions &LangOpts) {
   const CharSourceRange Range =
       CharSourceRange::getTokenRange(NodeToReplace->getSourceRange());
-  setFromSourceRange(Sources, Range, ReplacementText);
+  setFromSourceRange(Sources, Range, ReplacementText, LangOpts);
 }
 
 } // end namespace tooling

Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=238922&r1=238921&r2=238922&view=diff
==============================================================================
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Wed Jun  3 08:10:41 2015
@@ -43,8 +43,9 @@ Replacement::Replacement(const SourceMan
 
 Replacement::Replacement(const SourceManager &Sources,
                          const CharSourceRange &Range,
-                         StringRef ReplacementText) {
-  setFromSourceRange(Sources, Range, ReplacementText);
+                         StringRef ReplacementText,
+                         const LangOptions &LangOpts) {
+  setFromSourceRange(Sources, Range, ReplacementText, LangOpts);
 }
 
 bool Replacement::isApplicable() const {
@@ -124,23 +125,25 @@ void Replacement::setFromSourceLocation(
 // to handle ranges for refactoring in general first - there is no obvious
 // good way how to integrate this into the Lexer yet.
 static int getRangeSize(const SourceManager &Sources,
-                        const CharSourceRange &Range) {
+                        const CharSourceRange &Range,
+                        const LangOptions &LangOpts) {
   SourceLocation SpellingBegin = Sources.getSpellingLoc(Range.getBegin());
   SourceLocation SpellingEnd = Sources.getSpellingLoc(Range.getEnd());
   std::pair<FileID, unsigned> Start = Sources.getDecomposedLoc(SpellingBegin);
   std::pair<FileID, unsigned> End = Sources.getDecomposedLoc(SpellingEnd);
   if (Start.first != End.first) return -1;
   if (Range.isTokenRange())
-    End.second += Lexer::MeasureTokenLength(SpellingEnd, Sources,
-                                            LangOptions());
+    End.second += Lexer::MeasureTokenLength(SpellingEnd, Sources, LangOpts);
   return End.second - Start.second;
 }
 
 void Replacement::setFromSourceRange(const SourceManager &Sources,
                                      const CharSourceRange &Range,
-                                     StringRef ReplacementText) {
+                                     StringRef ReplacementText,
+                                     const LangOptions &LangOpts) {
   setFromSourceLocation(Sources, Sources.getSpellingLoc(Range.getBegin()),
-                        getRangeSize(Sources, Range), ReplacementText);
+                        getRangeSize(Sources, Range, LangOpts),
+                        ReplacementText);
 }
 
 unsigned shiftedCodePosition(const Replacements &Replaces, unsigned Position) {

Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=238922&r1=238921&r2=238922&view=diff
==============================================================================
--- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Wed Jun  3 08:10:41 2015
@@ -281,6 +281,7 @@ public:
 
 protected:
   clang::SourceManager *SM;
+  clang::ASTContext *Context;
 
 private:
   class FindConsumer : public clang::ASTConsumer {
@@ -303,6 +304,7 @@ private:
     CreateASTConsumer(clang::CompilerInstance &compiler,
                       llvm::StringRef dummy) override {
       Visitor->SM = &compiler.getSourceManager();
+      Visitor->Context = &compiler.getASTContext();
       /// TestConsumer will be deleted by the framework calling us.
       return llvm::make_unique<FindConsumer>(Visitor);
     }
@@ -368,6 +370,29 @@ TEST(Replacement, TemplatedFunctionCall)
   expectReplacementAt(CallToF.Replace, "input.cc", 43, 8);
 }
 
+class NestedNameSpecifierAVisitor
+    : public TestVisitor<NestedNameSpecifierAVisitor> {
+public:
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLoc) {
+    if (NNSLoc.getNestedNameSpecifier()) {
+      if (const NamespaceDecl* NS = NNSLoc.getNestedNameSpecifier()->getAsNamespace()) {
+        if (NS->getName() == "a") {
+          Replace = Replacement(*SM, &NNSLoc, "", Context->getLangOpts());
+        }
+      }
+    }
+    return TestVisitor<NestedNameSpecifierAVisitor>::TraverseNestedNameSpecifierLoc(
+        NNSLoc);
+  }
+  Replacement Replace;
+};
+
+TEST(Replacement, ColonColon) {
+  NestedNameSpecifierAVisitor VisitNNSA;
+  EXPECT_TRUE(VisitNNSA.runOver("namespace a { void f() { ::a::f(); } }"));
+  expectReplacementAt(VisitNNSA.Replace, "input.cc", 25, 5);
+}
+
 TEST(Range, overlaps) {
   EXPECT_TRUE(Range(10, 10).overlapsWith(Range(0, 11)));
   EXPECT_TRUE(Range(0, 11).overlapsWith(Range(10, 10)));





More information about the cfe-commits mailing list