[change-namespace] handle constructor initializer: Derived : Base::Base() {} and added conflict detections
Summary:
namespace nx { namespace ny { class Base { public: Base(i) {}} } }
namespace na {
namespace nb {
class X : public nx::ny {
public:
X() : Base::Base(1) {}
};
}
}
When changing from na::nb to x::y, "Base::Base" will be changed to "nx::ny::Base" and
"Base::" in "Base::Base" will be replaced with "nx::ny::Base" too, which causes
conflict. This conflict should've been detected when adding replacements but was hidden by `addOrMergeReplacement`. We now also detect conflict when adding replacements where conflict must not happen.
The namespace lookup is tricky here, we simply replace "Base::Base()" with "nx::ny::Base()" as a workaround, which compiles but not perfect.
Reviewers: hokein
Subscribers: bkramer, cfe-commits
Differential Revision: https://reviews.llvm.org/D26637
llvm-svn: 287118
diff --git a/clang-tools-extra/change-namespace/ChangeNamespace.cpp b/clang-tools-extra/change-namespace/ChangeNamespace.cpp
index af29c6d..fbbde43 100644
--- a/clang-tools-extra/change-namespace/ChangeNamespace.cpp
+++ b/clang-tools-extra/change-namespace/ChangeNamespace.cpp
@@ -9,6 +9,7 @@
#include "ChangeNamespace.h"
#include "clang/Format/Format.h"
#include "clang/Lex/Lexer.h"
+#include "llvm/Support/ErrorHandling.h"
using namespace clang::ast_matchers;
@@ -336,14 +337,27 @@
.bind("using_with_shadow"),
this);
- // Handle types in nested name specifier.
+ // Handle types in nested name specifier. Specifiers that are in a TypeLoc
+ // matched above are not matched, e.g. "A::" in "A::A" is not matched since
+ // "A::A" would have already been fixed.
Finder->addMatcher(nestedNameSpecifierLoc(
hasAncestor(decl(IsInMovedNs).bind("dc")),
loc(nestedNameSpecifier(specifiesType(
- hasDeclaration(DeclMatcher.bind("from_decl"))))))
+ hasDeclaration(DeclMatcher.bind("from_decl"))))),
+ unless(hasAncestor(typeLoc(loc(qualType(hasDeclaration(
+ decl(equalsBoundNode("from_decl")))))))))
.bind("nested_specifier_loc"),
this);
+ // Matches base class initializers in constructors. TypeLocs of base class
+ // initializers do not need to be fixed. For example,
+ // class X : public a::b::Y {
+ // public:
+ // X() : Y::Y() {} // Y::Y do not need namespace specifier.
+ // };
+ Finder->addMatcher(
+ cxxCtorInitializer(isBaseInitializer()).bind("base_initializer"), this);
+
// Handle function.
// Only handle functions that are defined in a namespace excluding member
// function, static methods (qualified by nested specifier), and functions
@@ -393,6 +407,11 @@
SourceLocation Start = Specifier->getBeginLoc();
SourceLocation End = EndLocationForType(Specifier->getTypeLoc());
fixTypeLoc(Result, Start, End, Specifier->getTypeLoc());
+ } else if (const auto *BaseInitializer =
+ Result.Nodes.getNodeAs<CXXCtorInitializer>(
+ "base_initializer")) {
+ BaseCtorInitializerTypeLocs.push_back(
+ BaseInitializer->getTypeSourceInfo()->getTypeLoc());
} else if (const auto *TLoc = Result.Nodes.getNodeAs<TypeLoc>("type")) {
fixTypeLoc(Result, startLocationForType(*TLoc), EndLocationForType(*TLoc),
*TLoc);
@@ -520,7 +539,9 @@
// Delete the forward declaration from the code to be moved.
const auto Deletion =
createReplacement(Start, End, "", *Result.SourceManager);
- addOrMergeReplacement(Deletion, &FileToReplacements[Deletion.getFilePath()]);
+ auto Err = FileToReplacements[Deletion.getFilePath()].add(Deletion);
+ if (Err)
+ llvm_unreachable(llvm::toString(std::move(Err)).c_str());
llvm::StringRef Code = Lexer::getSourceText(
CharSourceRange::getTokenRange(
Result.SourceManager->getSpellingLoc(Start),
@@ -606,7 +627,9 @@
if (NestedName == ReplaceName)
return;
auto R = createReplacement(Start, End, ReplaceName, *Result.SourceManager);
- addOrMergeReplacement(R, &FileToReplacements[R.getFilePath()]);
+ auto Err = FileToReplacements[R.getFilePath()].add(R);
+ if (Err)
+ llvm_unreachable(llvm::toString(std::move(Err)).c_str());
}
// Replace the [Start, End] of `Type` with the shortest qualified name when the
@@ -617,6 +640,9 @@
// FIXME: do not rename template parameter.
if (Start.isInvalid() || End.isInvalid())
return;
+ // Types of CXXCtorInitializers do not need to be fixed.
+ if (llvm::is_contained(BaseCtorInitializerTypeLocs, Type))
+ return;
// The declaration which this TypeLoc refers to.
const auto *FromDecl = Result.Nodes.getNodeAs<NamedDecl>("from_decl");
// `hasDeclaration` gives underlying declaration, but if the type is
@@ -667,7 +693,9 @@
// Use fully qualified name in UsingDecl for now.
auto R = createReplacement(Start, End, "using ::" + TargetDeclName,
*Result.SourceManager);
- addOrMergeReplacement(R, &FileToReplacements[R.getFilePath()]);
+ auto Err = FileToReplacements[R.getFilePath()].add(R);
+ if (Err)
+ llvm_unreachable(llvm::toString(std::move(Err)).c_str());
}
void ChangeNamespaceTool::onEndOfTranslationUnit() {