ioeric created this revision.
ioeric added a reviewer: hokein.
ioeric added a subscriber: cfe-commits.
when replacing symbol references in moved namespaces, trying to make the replace
name as short as possible by considering UsingDecl (i.e. UsingShadow) and
UsingDirectiveDecl (i.e. using namespace decl).
https://reviews.llvm.org/D25771
Files:
change-namespace/ChangeNamespace.cpp
change-namespace/ChangeNamespace.h
unittests/change-namespace/ChangeNamespaceTests.cpp
Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===================================================================
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -272,8 +272,8 @@
"}\n"
"namespace nb {\n"
"using nc::SAME;\n"
- "using YO = nc::SAME;\n"
- "typedef nc::SAME IDENTICAL;\n"
+ "using YO = nd::SAME;\n"
+ "typedef nd::SAME IDENTICAL;\n"
"void f(nd::SAME Same) {}\n"
"} // namespace nb\n"
"} // namespace na\n";
@@ -292,93 +292,14 @@
"namespace x {\n"
"namespace y {\n"
"using ::na::nc::SAME;\n"
- "using YO = na::nc::SAME;\n"
- "typedef na::nc::SAME IDENTICAL;\n"
+ "using YO = na::nd::SAME;\n"
+ "typedef na::nd::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, UsingShadowDeclInFunction) {
- std::string Code = "namespace glob {\n"
- "class Glob {};\n"
- "}\n"
- "namespace na {\n"
- "namespace nb {\n"
- "void f() {\n"
- " using glob::Glob;\n"
- " Glob g;\n"
- "}\n"
- "} // namespace nb\n"
- "} // namespace na\n";
-
- // FIXME: don't add namespace qualifier when there is UsingShadowDecl.
- std::string Expected = "namespace glob {\n"
- "class Glob {};\n"
- "}\n"
- "\n"
- "namespace x {\n"
- "namespace y {\n"
- "void f() {\n"
- " using ::glob::Glob;\n"
- " glob::Glob g;\n"
- "}\n"
- "} // namespace y\n"
- "} // namespace x\n";
- EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
-}
-
-TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
- std::string Code = "namespace glob {\n"
- "class Glob {};\n"
- "}\n"
- "using glob::Glob;\n"
- "namespace na {\n"
- "namespace nb {\n"
- "void f() { Glob g; }\n"
- "} // namespace nb\n"
- "} // namespace na\n";
-
- // FIXME: don't add namespace qualifier when there is UsingShadowDecl.
- std::string Expected = "namespace glob {\n"
- "class Glob {};\n"
- "}\n"
- "using glob::Glob;\n"
- "\n"
- "namespace x {\n"
- "namespace y {\n"
- "void f() { glob::Glob g; }\n"
- "} // namespace y\n"
- "} // namespace x\n";
- EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
-}
-
-TEST_F(ChangeNamespaceTest, UsingNamespace) {
- std::string Code = "namespace glob {\n"
- "class Glob {};\n"
- "}\n"
- "using namespace glob;\n"
- "namespace na {\n"
- "namespace nb {\n"
- "void f() { Glob g; }\n"
- "} // namespace nb\n"
- "} // namespace na\n";
-
- // FIXME: don't add namespace qualifier when there is "using namespace" decl.
- std::string Expected = "namespace glob {\n"
- "class Glob {};\n"
- "}\n"
- "using namespace glob;\n"
- "\n"
- "namespace x {\n"
- "namespace y {\n"
- "void f() { glob::Glob g; }\n"
- "} // namespace y\n"
- "} // namespace x\n";
- EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
-}
-
TEST_F(ChangeNamespaceTest, TypeInNestedNameSpecifier) {
std::string Code =
"namespace na {\n"
@@ -584,6 +505,237 @@
EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
}
+TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
+ std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "using glob::Glob;\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "void f() { Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+ std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "using glob::Glob;\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() { Glob g; }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingNamespaceInGlobal) {
+ std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "using namespace glob;\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "void f() { Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+ std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "using namespace glob;\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() { Glob g; }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingDeclAfterReference) {
+ std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "void f() { glob::Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n"
+ "using glob::Glob;\n"
+ "using namespace glob;\n";
+
+ std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() { glob::Glob g; }\n"
+ "} // namespace y\n"
+ "} // namespace x\n"
+ "using glob::Glob;\n"
+ "using namespace glob;\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingNamespaceAndUsingShadowInGlobal) {
+ std::string Code = "namespace glob1 {\n"
+ "namespace glob2 {\n"
+ "class Glob {};\n"
+ "}\n"
+ "}\n"
+ "using glob1::glob2::Glob;\n"
+ "using namespace glob1;\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "void f() { Glob g; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+ std::string Expected = "namespace glob1 {\n"
+ "namespace glob2 {\n"
+ "class Glob {};\n"
+ "}\n"
+ "}\n"
+ "using glob1::glob2::Glob;\n"
+ "using namespace glob1;\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() { Glob g; }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingAliasInGlobal) {
+ std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "using GLB = glob::Glob;\n"
+ "using BLG = glob::Glob;\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "void f() { GLB g; BLG blg; }\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+ std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "using GLB = glob::Glob;\n"
+ "using BLG = glob::Glob;\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() { GLB g; BLG blg; }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclAndMovedNamespace) {
+ std::string Code = "namespace na { class C_A {};\n }\n"
+ "using na::C_A;\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "class C_X {\n"
+ "public:\n"
+ " C_A a;\n"
+ "};\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+ std::string Expected = "namespace na { class C_A {};\n }\n"
+ "using na::C_A;\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "class C_X {\n"
+ "public:\n"
+ " C_A a;\n"
+ "};\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingNamespaceDeclAndMovedNamespace) {
+ std::string Code = "namespace na { class C_A {};\n }\n"
+ "using namespace na;\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "class C_X {\n"
+ "public:\n"
+ " C_A ca;\n"
+ "};\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+ std::string Expected = "namespace na { class C_A {};\n }\n"
+ "using namespace na;\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "class C_X {\n"
+ "public:\n"
+ " C_A ca;\n"
+ "};\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclInFunction) {
+ std::string Code = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "void f() {\n"
+ " using glob::Glob;\n"
+ " Glob g;\n"
+ "}\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+ std::string Expected = "namespace glob {\n"
+ "class Glob {};\n"
+ "}\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() {\n"
+ " using ::glob::Glob;\n"
+ " Glob g;\n"
+ "}\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclInClass) {
+ std::string Code = "namespace na { class C_A {};\n }\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "void f() {\n"
+ " using na::CA;\n"
+ " CA ca;\n"
+ "}\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+ std::string Expected = "namespace na { class C_A {};\n }\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "void f() {\n"
+ " using na::CA;\n"
+ " CA ca;\n"
+ "}\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
} // anonymous namespace
} // namespace change_namespace
} // namespace clang
Index: change-namespace/ChangeNamespace.h
===================================================================
--- change-namespace/ChangeNamespace.h
+++ change-namespace/ChangeNamespace.h
@@ -67,8 +67,8 @@
void replaceQualifiedSymbolInDeclContext(
const ast_matchers::MatchFinder::MatchResult &Result,
- const Decl *DeclContext, SourceLocation Start, SourceLocation End,
- llvm::StringRef DeclName);
+ const DeclContext *DeclContext, SourceLocation Start, SourceLocation End,
+ const NamedDecl *FromDecl);
void fixTypeLoc(const ast_matchers::MatchFinder::MatchResult &Result,
SourceLocation Start, SourceLocation End, TypeLoc Type);
@@ -139,6 +139,12 @@
// will be done after removing the code from the old namespace and before
// inserting it to the new namespace.
std::map<std::string, std::vector<InsertForwardDeclaration>> InsertFwdDecls;
+ // Records all using declarations, which can be used to shorten namespace
+ // specifiers.
+ llvm::SmallPtrSet<const UsingDecl *, 8> UsingDecls;
+ // Records all using namespace declarations, which can be used to shorten
+ // namespace specifiers.
+ llvm::SmallPtrSet<const UsingDirectiveDecl *, 8> UsingNamespaceDecls;
};
} // namespace change_namespace
Index: change-namespace/ChangeNamespace.cpp
===================================================================
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -189,10 +189,8 @@
llvm::StringRef NsName) {
DeclName = DeclName.ltrim(':');
NsName = NsName.ltrim(':');
- // If `DeclName` is a global variable, we prepend "::" to it if it is not in
- // the global namespace.
if (DeclName.find(':') == llvm::StringRef::npos)
- return NsName.empty() ? DeclName.str() : ("::" + DeclName).str();
+ return DeclName;
while (!DeclName.consume_front((NsName + "::").str())) {
const auto Pos = NsName.find_last_of(':');
@@ -219,6 +217,26 @@
return Code;
}
+// Returns true if \p D is a nested DeclContext in \p Context
+bool isNestedDeclContext(const DeclContext *D, const DeclContext *Context) {
+ while (D) {
+ if (D == Context)
+ return true;
+ D = D->getParent();
+ }
+ return false;
+}
+
+// Returns true if \p D is visible at \p Loc with DeclContext \p DeclCtx.
+bool isDeclVisibleAtLocation(const SourceManager &SM, const Decl *D,
+ const DeclContext *DeclCtx, SourceLocation Loc) {
+ return SM.isBeforeInTranslationUnit(SM.getSpellingLoc(D->getLocation()),
+ SM.getSpellingLoc(Loc)) &&
+ (SM.getFileID(SM.getSpellingLoc(D->getLocation())) ==
+ SM.getFileID(SM.getSpellingLoc(Loc))) &&
+ isNestedDeclContext(DeclCtx, D->getDeclContext());
+}
+
} // anonymous namespace
ChangeNamespaceTool::ChangeNamespaceTool(
@@ -244,6 +262,14 @@
}
void ChangeNamespaceTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
+ // Match using declarations.
+ Finder->addMatcher(
+ usingDecl(isExpansionInFileMatching(FilePattern)).bind("using"), this);
+ // Match using namespace declarations.
+ Finder->addMatcher(usingDirectiveDecl(isExpansionInFileMatching(FilePattern))
+ .bind("using_namespace"),
+ this);
+
// Match old namespace blocks.
std::string FullOldNs = "::" + OldNamespace;
Finder->addMatcher(
@@ -286,9 +312,9 @@
// Types in `UsingShadowDecl` is not matched by `typeLoc` above, so we need to
// special case it.
- Finder->addMatcher(
- usingDecl(IsInMovedNs, hasAnyUsingShadowDecl(decl())).bind("using_decl"),
- this);
+ Finder->addMatcher(usingDecl(IsInMovedNs, hasAnyUsingShadowDecl(decl()))
+ .bind("using_with_shadow"),
+ this);
// Handle types in nested name specifier.
Finder->addMatcher(nestedNameSpecifierLoc(
@@ -326,14 +352,21 @@
void ChangeNamespaceTool::run(
const ast_matchers::MatchFinder::MatchResult &Result) {
- if (const auto *NsDecl = Result.Nodes.getNodeAs<NamespaceDecl>("old_ns")) {
+ if (const auto *Using = Result.Nodes.getNodeAs<UsingDecl>("using")) {
+ UsingDecls.insert(Using);
+ } else if (const auto *UsingNamespace =
+ Result.Nodes.getNodeAs<UsingDirectiveDecl>(
+ "using_namespace")) {
+ UsingNamespaceDecls.insert(UsingNamespace);
+ } else if (const auto *NsDecl =
+ Result.Nodes.getNodeAs<NamespaceDecl>("old_ns")) {
moveOldNamespace(Result, NsDecl);
} else if (const auto *FwdDecl =
Result.Nodes.getNodeAs<CXXRecordDecl>("fwd_decl")) {
moveClassForwardDeclaration(Result, FwdDecl);
- } else if (const auto *UsingDeclaration =
- Result.Nodes.getNodeAs<UsingDecl>("using_decl")) {
- fixUsingShadowDecl(Result, UsingDeclaration);
+ } else if (const auto *UsingWithShadow =
+ Result.Nodes.getNodeAs<UsingDecl>("using_with_shadow")) {
+ fixUsingShadowDecl(Result, UsingWithShadow);
} else if (const auto *Specifier =
Result.Nodes.getNodeAs<NestedNameSpecifierLoc>(
"nested_specifier_loc")) {
@@ -343,17 +376,18 @@
} else if (const auto *TLoc = Result.Nodes.getNodeAs<TypeLoc>("type")) {
fixTypeLoc(Result, startLocationForType(*TLoc), EndLocationForType(*TLoc),
*TLoc);
- } else if (const auto *VarRef = Result.Nodes.getNodeAs<DeclRefExpr>("var_ref")){
+ } else if (const auto *VarRef =
+ Result.Nodes.getNodeAs<DeclRefExpr>("var_ref")) {
const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var_decl");
assert(Var);
if (Var->getCanonicalDecl()->isStaticDataMember())
return;
- std::string Name = Var->getQualifiedNameAsString();
const clang::Decl *Context = Result.Nodes.getNodeAs<clang::Decl>("dc");
assert(Context && "Empty decl context.");
clang::SourceRange VarRefRange = VarRef->getSourceRange();
- replaceQualifiedSymbolInDeclContext(Result, Context, VarRefRange.getBegin(),
- VarRefRange.getEnd(), Name);
+ replaceQualifiedSymbolInDeclContext(
+ Result, Context->getDeclContext(), VarRefRange.getBegin(),
+ VarRefRange.getEnd(), llvm::dyn_cast<NamedDecl>(Var));
} else {
const auto *Call = Result.Nodes.getNodeAs<clang::CallExpr>("call");
assert(Call != nullptr &&"Expecting callback for CallExpr.");
@@ -365,12 +399,12 @@
clang::StorageClass::SC_Static &&
Func->isOutOfLine())
return;
- std::string Name = Func->getQualifiedNameAsString();
const clang::Decl *Context = Result.Nodes.getNodeAs<clang::Decl>("dc");
assert(Context && "Empty decl context.");
clang::SourceRange CalleeRange = Call->getCallee()->getSourceRange();
- replaceQualifiedSymbolInDeclContext(Result, Context, CalleeRange.getBegin(),
- CalleeRange.getEnd(), Name);
+ replaceQualifiedSymbolInDeclContext(
+ Result, Context->getDeclContext(), CalleeRange.getBegin(),
+ CalleeRange.getEnd(), llvm::dyn_cast<NamedDecl>(Func));
}
}
@@ -484,15 +518,14 @@
InsertFwdDecls[Insertion.getFilePath()].push_back(InsertFwd);
}
-// Replaces a qualified symbol that refers to a declaration `DeclName` with the
-// shortest qualified name possible when the reference is in `NewNamespace`.
-// FIXME: don't need to add redundant namespace qualifier when there is
-// UsingShadowDecl or using namespace decl.
+// Replaces a qualified symbol (in \p DeclCtx) that refers to a declaration \p
+// FromDecl with the shortest qualified name possible when the reference is in
+// `NewNamespace`.
void ChangeNamespaceTool::replaceQualifiedSymbolInDeclContext(
- const ast_matchers::MatchFinder::MatchResult &Result, const Decl *DeclCtx,
- SourceLocation Start, SourceLocation End, llvm::StringRef DeclName) {
- const auto *NsDeclContext =
- DeclCtx->getDeclContext()->getEnclosingNamespaceContext();
+ const ast_matchers::MatchFinder::MatchResult &Result,
+ const DeclContext *DeclCtx, SourceLocation Start, SourceLocation End,
+ const NamedDecl *FromDecl) {
+ const auto *NsDeclContext = DeclCtx->getEnclosingNamespaceContext();
const auto *NsDecl = llvm::dyn_cast<NamespaceDecl>(NsDeclContext);
// Calculate the name of the `NsDecl` after it is moved to new namespace.
std::string OldNs = NsDecl->getQualifiedNameAsString();
@@ -510,8 +543,37 @@
// If the symbol is already fully qualified, no change needs to be make.
if (NestedName.startswith("::"))
return;
+ std::string FromDeclName = FromDecl->getQualifiedNameAsString();
std::string ReplaceName =
- getShortestQualifiedNameInNamespace(DeclName, NewNs);
+ getShortestQualifiedNameInNamespace(FromDeclName, NewNs);
+ // Checks if there is any using namespace declarations that can shorten the
+ // qualified name.
+ for (const auto *UsingNamespace : UsingNamespaceDecls) {
+ StringRef FromDeclNameRef = FromDeclName;
+ if (FromDeclNameRef.consume_front(UsingNamespace->getNominatedNamespace()
+ ->getQualifiedNameAsString())) {
+ FromDeclNameRef = FromDeclNameRef.drop_front(2);
+ if (FromDeclNameRef.size() < ReplaceName.size())
+ ReplaceName = FromDeclNameRef;
+ }
+ }
+ // Checks if there is any using shadow declarations that can shorten the
+ // qualified name.
+ bool Matched = false;
+ for (const UsingDecl *Using : UsingDecls) {
+ if (Matched)
+ break;
+ if (isDeclVisibleAtLocation(*Result.SourceManager, Using, DeclCtx, Start)) {
+ for (const auto *UsingShadow : Using->shadows()) {
+ const auto *TargetDecl = UsingShadow->getTargetDecl();
+ if (TargetDecl == FromDecl) {
+ ReplaceName = FromDecl->getNameAsString();
+ Matched = true;
+ break;
+ }
+ }
+ }
+ }
// If the new nested name in the new namespace is the same as it was in the
// old namespace, we don't create replacement.
if (NestedName == ReplaceName)
@@ -537,8 +599,8 @@
const Decl *DeclCtx = Result.Nodes.getNodeAs<Decl>("dc");
assert(DeclCtx && "Empty decl context.");
- replaceQualifiedSymbolInDeclContext(Result, DeclCtx, Start, End,
- FromDecl->getQualifiedNameAsString());
+ replaceQualifiedSymbolInDeclContext(Result, DeclCtx->getDeclContext(), Start,
+ End, FromDecl);
}
void ChangeNamespaceTool::fixUsingShadowDecl(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
