Index: clang-tools-extra/trunk/change-namespace/ChangeNamespace.h =================================================================== --- clang-tools-extra/trunk/change-namespace/ChangeNamespace.h +++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.h @@ -73,6 +73,9 @@ void fixTypeLoc(const ast_matchers::MatchFinder::MatchResult &Result, SourceLocation Start, SourceLocation End, TypeLoc Type); + void fixUsingShadowDecl(const ast_matchers::MatchFinder::MatchResult &Result, + const UsingDecl *UsingDeclaration); + // Information about moving an old namespace. struct MoveNamespace { // The start offset of the namespace block being moved in the original Index: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp =================================================================== --- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp +++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp @@ -231,9 +231,6 @@ } // FIXME: handle the following symbols: -// - Types in `UsingShadowDecl` (e.g. `using a::b::c;`) which are not matched -// by `typeLoc`. -// - Types in nested name specifier, e.g. "na::X" in "na::X::Nested". // - Function references. // - Variable references. void ChangeNamespaceTool::registerMatchers(ast_matchers::MatchFinder *Finder) { @@ -266,7 +263,6 @@ // Match TypeLocs on the declaration. Carefully match only the outermost // TypeLoc that's directly linked to the old class and don't handle nested // name specifier locs. - // FIXME: match and handle nested name specifier locs. Finder->addMatcher( typeLoc(IsInMovedNs, loc(qualType(hasDeclaration(DeclMatcher.bind("from_decl")))), @@ -276,6 +272,17 @@ hasAncestor(decl().bind("dc"))) .bind("type"), this); + // Types in `UsingShadowDecl` is not matched by `typeLoc` above, so we need to + // special case it. + Finder->addMatcher( + usingDecl(hasAnyUsingShadowDecl(IsInMovedNs)).bind("using_decl"), this); + // Handle types in nested name specifier. + Finder->addMatcher(nestedNameSpecifierLoc( + hasAncestor(decl(IsInMovedNs).bind("dc")), + loc(nestedNameSpecifier(specifiesType( + hasDeclaration(DeclMatcher.bind("from_decl")))))) + .bind("nested_specifier_loc"), + this); } void ChangeNamespaceTool::run( @@ -285,6 +292,15 @@ } else if (const auto *FwdDecl = Result.Nodes.getNodeAs("fwd_decl")) { moveClassForwardDeclaration(Result, FwdDecl); + } else if (const auto *UsingDeclaration = + Result.Nodes.getNodeAs("using_decl")) { + fixUsingShadowDecl(Result, UsingDeclaration); + } else if (const auto *Specifier = + Result.Nodes.getNodeAs( + "nested_specifier_loc")) { + SourceLocation Start = Specifier->getBeginLoc(); + SourceLocation End = EndLocationForType(Specifier->getTypeLoc()); + fixTypeLoc(Result, Start, End, Specifier->getTypeLoc()); } else { const auto *TLoc = Result.Nodes.getNodeAs("type"); assert(TLoc != nullptr && "Expecting callback for TypeLoc"); @@ -439,6 +455,26 @@ FromDecl->getQualifiedNameAsString()); } +void ChangeNamespaceTool::fixUsingShadowDecl( + const ast_matchers::MatchFinder::MatchResult &Result, + const UsingDecl *UsingDeclaration) { + SourceLocation Start = UsingDeclaration->getLocStart(); + SourceLocation End = UsingDeclaration->getLocEnd(); + if (Start.isInvalid() || End.isInvalid()) return; + + assert(UsingDeclaration->shadow_size() > 0); + // FIXME: it might not be always accurate to use the first using-decl. + const NamedDecl *TargetDecl = + UsingDeclaration->shadow_begin()->getTargetDecl(); + std::string TargetDeclName = TargetDecl->getQualifiedNameAsString(); + // FIXME: check if target_decl_name is in moved ns, which doesn't make much + // sense. If this happens, we need to use name with the new namespace. + // Use fully qualified name in UsingDecl for now. + auto R = createReplacement(Start, End, "using ::" + TargetDeclName, + *Result.SourceManager); + addOrMergeReplacement(R, &FileToReplacements[R.getFilePath()]); +} + void ChangeNamespaceTool::onEndOfTranslationUnit() { // Move namespace blocks and insert forward declaration to old namespace. for (const auto &FileAndNsMoves : MoveNamespaces) { Index: clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp +++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp @@ -229,6 +229,91 @@ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); } +TEST_F(ChangeNamespaceTest, FixUsingShadowDecl) { + std::string Code = "namespace na {\n" + "namespace nc {\n" + "class SAME {};\n" + "}\n" + "namespace nd {\n" + "class SAME {};\n" + "}\n" + "namespace nb {\n" + "using nc::SAME;\n" + "using YO = nc::SAME;\n" + "typedef nc::SAME IDENTICAL;\n" + "void f(nd::SAME Same) {}\n" + "} // namespace nb\n" + "} // namespace na\n"; + + std::string Expected = "namespace na {\n" + "namespace nc {\n" + "class SAME {};\n" + "}\n" + "namespace nd {\n" + "class SAME {};\n" + "}\n" + "\n" + "} // namespace na\n" + "namespace x {\n" + "namespace y {\n" + "using ::na::nc::SAME;\n" + "using YO = na::nc::SAME;\n" + "typedef na::nc::SAME IDENTICAL;\n" + "void f(na::nd::SAME Same) {}\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + +TEST_F(ChangeNamespaceTest, TypeInNestedNameSpecifier) { + std::string Code = + "namespace na {\n" + "class C_A {\n" + "public:\n" + " class Nested {\n" + " public:\n" + " static int NestedX;\n" + " static void nestedFunc() {}\n" + " };\n" + "};\n" + "namespace nb {\n" + "class C_X {\n" + " C_A na;\n" + " C_A::Nested nested;\n" + " void f() {\n" + " C_A::Nested::nestedFunc();\n" + " int X = C_A::Nested::NestedX;\n" + " }\n" + "};\n" + "} // namespace nb\n" + "} // namespace na\n"; + std::string Expected = + "namespace na {\n" + "class C_A {\n" + "public:\n" + " class Nested {\n" + " public:\n" + " static int NestedX;\n" + " static void nestedFunc() {}\n" + " };\n" + "};\n" + "\n" + "} // namespace na\n" + "namespace x {\n" + "namespace y {\n" + "class C_X {\n" + " na::C_A na;\n" + " na::C_A::Nested nested;\n" + " void f() {\n" + " na::C_A::Nested::nestedFunc();\n" + " int X = na::C_A::Nested::NestedX;\n" + " }\n" + "};\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + } // anonymous namespace } // namespace change_namespace } // namespace clang