Skip to content
This repository was archived by the owner on Nov 1, 2021. It is now read-only.

Commit ee4a96d

Browse files
committed
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 "::". git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@238922 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 500d43f commit ee4a96d

File tree

3 files changed

+54
-20
lines changed

3 files changed

+54
-20
lines changed

Diff for: include/clang/Tooling/Core/Replacement.h

+19-13
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#ifndef LLVM_CLANG_TOOLING_CORE_REPLACEMENT_H
2020
#define LLVM_CLANG_TOOLING_CORE_REPLACEMENT_H
2121

22+
#include "clang/Basic/LangOptions.h"
2223
#include "clang/Basic/SourceLocation.h"
2324
#include "llvm/ADT/StringRef.h"
2425
#include <set>
@@ -77,22 +78,24 @@ class Replacement {
7778
/// \param FilePath A source file accessible via a SourceManager.
7879
/// \param Offset The byte offset of the start of the range in the file.
7980
/// \param Length The length of the range in bytes.
80-
Replacement(StringRef FilePath, unsigned Offset,
81-
unsigned Length, StringRef ReplacementText);
81+
Replacement(StringRef FilePath, unsigned Offset, unsigned Length,
82+
StringRef ReplacementText);
8283

8384
/// \brief Creates a Replacement of the range [Start, Start+Length) with
8485
/// ReplacementText.
85-
Replacement(const SourceManager &Sources, SourceLocation Start, unsigned Length,
86-
StringRef ReplacementText);
86+
Replacement(const SourceManager &Sources, SourceLocation Start,
87+
unsigned Length, StringRef ReplacementText);
8788

8889
/// \brief Creates a Replacement of the given range with ReplacementText.
8990
Replacement(const SourceManager &Sources, const CharSourceRange &Range,
90-
StringRef ReplacementText);
91+
StringRef ReplacementText,
92+
const LangOptions &LangOpts = LangOptions());
9193

9294
/// \brief Creates a Replacement of the node with ReplacementText.
9395
template <typename Node>
9496
Replacement(const SourceManager &Sources, const Node &NodeToReplace,
95-
StringRef ReplacementText);
97+
StringRef ReplacementText,
98+
const LangOptions &LangOpts = LangOptions());
9699

97100
/// \brief Returns whether this replacement can be applied to a file.
98101
///
@@ -114,11 +117,13 @@ class Replacement {
114117
std::string toString() const;
115118

116119
private:
117-
void setFromSourceLocation(const SourceManager &Sources, SourceLocation Start,
118-
unsigned Length, StringRef ReplacementText);
119-
void setFromSourceRange(const SourceManager &Sources,
120-
const CharSourceRange &Range,
121-
StringRef ReplacementText);
120+
void setFromSourceLocation(const SourceManager &Sources,
121+
SourceLocation Start, unsigned Length,
122+
StringRef ReplacementText);
123+
void setFromSourceRange(const SourceManager &Sources,
124+
const CharSourceRange &Range,
125+
StringRef ReplacementText,
126+
const LangOptions &LangOpts);
122127

123128
std::string FilePath;
124129
Range ReplacementRange;
@@ -217,10 +222,11 @@ std::string applyAllReplacements(StringRef Code, const Replacements &Replaces);
217222

218223
template <typename Node>
219224
Replacement::Replacement(const SourceManager &Sources,
220-
const Node &NodeToReplace, StringRef ReplacementText) {
225+
const Node &NodeToReplace, StringRef ReplacementText,
226+
const LangOptions &LangOpts) {
221227
const CharSourceRange Range =
222228
CharSourceRange::getTokenRange(NodeToReplace->getSourceRange());
223-
setFromSourceRange(Sources, Range, ReplacementText);
229+
setFromSourceRange(Sources, Range, ReplacementText, LangOpts);
224230
}
225231

226232
} // end namespace tooling

Diff for: lib/Tooling/Core/Replacement.cpp

+10-7
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@ Replacement::Replacement(const SourceManager &Sources, SourceLocation Start,
4343

4444
Replacement::Replacement(const SourceManager &Sources,
4545
const CharSourceRange &Range,
46-
StringRef ReplacementText) {
47-
setFromSourceRange(Sources, Range, ReplacementText);
46+
StringRef ReplacementText,
47+
const LangOptions &LangOpts) {
48+
setFromSourceRange(Sources, Range, ReplacementText, LangOpts);
4849
}
4950

5051
bool Replacement::isApplicable() const {
@@ -124,23 +125,25 @@ void Replacement::setFromSourceLocation(const SourceManager &Sources,
124125
// to handle ranges for refactoring in general first - there is no obvious
125126
// good way how to integrate this into the Lexer yet.
126127
static int getRangeSize(const SourceManager &Sources,
127-
const CharSourceRange &Range) {
128+
const CharSourceRange &Range,
129+
const LangOptions &LangOpts) {
128130
SourceLocation SpellingBegin = Sources.getSpellingLoc(Range.getBegin());
129131
SourceLocation SpellingEnd = Sources.getSpellingLoc(Range.getEnd());
130132
std::pair<FileID, unsigned> Start = Sources.getDecomposedLoc(SpellingBegin);
131133
std::pair<FileID, unsigned> End = Sources.getDecomposedLoc(SpellingEnd);
132134
if (Start.first != End.first) return -1;
133135
if (Range.isTokenRange())
134-
End.second += Lexer::MeasureTokenLength(SpellingEnd, Sources,
135-
LangOptions());
136+
End.second += Lexer::MeasureTokenLength(SpellingEnd, Sources, LangOpts);
136137
return End.second - Start.second;
137138
}
138139

139140
void Replacement::setFromSourceRange(const SourceManager &Sources,
140141
const CharSourceRange &Range,
141-
StringRef ReplacementText) {
142+
StringRef ReplacementText,
143+
const LangOptions &LangOpts) {
142144
setFromSourceLocation(Sources, Sources.getSpellingLoc(Range.getBegin()),
143-
getRangeSize(Sources, Range), ReplacementText);
145+
getRangeSize(Sources, Range, LangOpts),
146+
ReplacementText);
144147
}
145148

146149
unsigned shiftedCodePosition(const Replacements &Replaces, unsigned Position) {

Diff for: unittests/Tooling/RefactoringTest.cpp

+25
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ class TestVisitor : public clang::RecursiveASTVisitor<T> {
281281

282282
protected:
283283
clang::SourceManager *SM;
284+
clang::ASTContext *Context;
284285

285286
private:
286287
class FindConsumer : public clang::ASTConsumer {
@@ -303,6 +304,7 @@ class TestVisitor : public clang::RecursiveASTVisitor<T> {
303304
CreateASTConsumer(clang::CompilerInstance &compiler,
304305
llvm::StringRef dummy) override {
305306
Visitor->SM = &compiler.getSourceManager();
307+
Visitor->Context = &compiler.getASTContext();
306308
/// TestConsumer will be deleted by the framework calling us.
307309
return llvm::make_unique<FindConsumer>(Visitor);
308310
}
@@ -368,6 +370,29 @@ TEST(Replacement, TemplatedFunctionCall) {
368370
expectReplacementAt(CallToF.Replace, "input.cc", 43, 8);
369371
}
370372

373+
class NestedNameSpecifierAVisitor
374+
: public TestVisitor<NestedNameSpecifierAVisitor> {
375+
public:
376+
bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLoc) {
377+
if (NNSLoc.getNestedNameSpecifier()) {
378+
if (const NamespaceDecl* NS = NNSLoc.getNestedNameSpecifier()->getAsNamespace()) {
379+
if (NS->getName() == "a") {
380+
Replace = Replacement(*SM, &NNSLoc, "", Context->getLangOpts());
381+
}
382+
}
383+
}
384+
return TestVisitor<NestedNameSpecifierAVisitor>::TraverseNestedNameSpecifierLoc(
385+
NNSLoc);
386+
}
387+
Replacement Replace;
388+
};
389+
390+
TEST(Replacement, ColonColon) {
391+
NestedNameSpecifierAVisitor VisitNNSA;
392+
EXPECT_TRUE(VisitNNSA.runOver("namespace a { void f() { ::a::f(); } }"));
393+
expectReplacementAt(VisitNNSA.Replace, "input.cc", 25, 5);
394+
}
395+
371396
TEST(Range, overlaps) {
372397
EXPECT_TRUE(Range(10, 10).overlapsWith(Range(0, 11)));
373398
EXPECT_TRUE(Range(0, 11).overlapsWith(Range(10, 10)));

0 commit comments

Comments
 (0)