From ff51f011d1edb2de79a2448f68e9de1788596184 Mon Sep 17 00:00:00 2001 From: Eric Liu Date: Wed, 16 Nov 2016 16:54:53 +0000 Subject: [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 --- .../change-namespace/ChangeNamespace.cpp | 38 +++++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) (limited to 'clang-tools-extra/change-namespace/ChangeNamespace.cpp') diff --git a/clang-tools-extra/change-namespace/ChangeNamespace.cpp b/clang-tools-extra/change-namespace/ChangeNamespace.cpp index af29c6d2947..fbbde4397cc 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 @@ void ChangeNamespaceTool::registerMatchers(ast_matchers::MatchFinder *Finder) { .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 @@ void ChangeNamespaceTool::run( SourceLocation Start = Specifier->getBeginLoc(); SourceLocation End = EndLocationForType(Specifier->getTypeLoc()); fixTypeLoc(Result, Start, End, Specifier->getTypeLoc()); + } else if (const auto *BaseInitializer = + Result.Nodes.getNodeAs( + "base_initializer")) { + BaseCtorInitializerTypeLocs.push_back( + BaseInitializer->getTypeSourceInfo()->getTypeLoc()); } else if (const auto *TLoc = Result.Nodes.getNodeAs("type")) { fixTypeLoc(Result, startLocationForType(*TLoc), EndLocationForType(*TLoc), *TLoc); @@ -520,7 +539,9 @@ void ChangeNamespaceTool::moveClassForwardDeclaration( // 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 @@ void ChangeNamespaceTool::replaceQualifiedSymbolInDeclContext( 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 @@ void ChangeNamespaceTool::fixTypeLoc( // 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("from_decl"); // `hasDeclaration` gives underlying declaration, but if the type is @@ -667,7 +693,9 @@ void ChangeNamespaceTool::fixUsingShadowDecl( // 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() { -- cgit v1.2.3