[clang-tools-extra] r288919 - [change-namespace] don't fix using shadow decls in classes.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Dec  7 11:04:07 2016
New Revision: 288919

URL: http://llvm.org/viewvc/llvm-project?rev=288919&view=rev
Log:
[change-namespace] don't fix using shadow decls in classes.

Summary:
Using shadow declarations in classes always refers to base class, which does not
need to be fixed/qualified since it can be inferred from inheritance.

Reviewers: bkramer

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D27523

Modified:
    clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
    clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp

Modified: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp?rev=288919&r1=288918&r2=288919&view=diff
==============================================================================
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp (original)
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp Wed Dec  7 11:04:07 2016
@@ -327,6 +327,12 @@ void ChangeNamespaceTool::registerMatche
           hasAncestor(cxxRecordDecl()),
           allOf(IsInMovedNs, unless(cxxRecordDecl(unless(isDefinition())))))));
 
+  // Using shadow declarations in classes always refers to base class, which
+  // does not need to be qualified since it can be inferred from inheritance.
+  // Note that this does not match using alias declarations.
+  auto UsingShadowDeclInClass =
+      usingDecl(hasAnyUsingShadowDecl(decl()), hasParent(cxxRecordDecl()));
+
   // Match TypeLocs on the declaration. Carefully match only the outermost
   // TypeLoc and template specialization arguments (which are not outermost)
   // that are directly linked to types matching `DeclMatcher`. Nested name
@@ -337,28 +343,36 @@ void ChangeNamespaceTool::registerMatche
               unless(anyOf(hasParent(typeLoc(loc(qualType(
                                allOf(hasDeclaration(DeclMatcher),
                                      unless(templateSpecializationType())))))),
-                           hasParent(nestedNameSpecifierLoc()))),
+                           hasParent(nestedNameSpecifierLoc()),
+                           hasAncestor(isImplicit()),
+                           hasAncestor(UsingShadowDeclInClass))),
               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(IsInMovedNs, hasAnyUsingShadowDecl(decl()))
+  // Since using declarations inside classes must have the base class in the
+  // nested name specifier, we leave it to the nested name specifier matcher.
+  Finder->addMatcher(usingDecl(IsInMovedNs, hasAnyUsingShadowDecl(decl()),
+                               unless(UsingShadowDeclInClass))
                          .bind("using_with_shadow"),
                      this);
 
   // 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"))))),
-                         unless(hasAncestor(typeLoc(loc(qualType(hasDeclaration(
-                             decl(equalsBoundNode("from_decl")))))))))
-                         .bind("nested_specifier_loc"),
-                     this);
+  Finder->addMatcher(
+      nestedNameSpecifierLoc(
+          hasAncestor(decl(IsInMovedNs).bind("dc")),
+          loc(nestedNameSpecifier(
+              specifiesType(hasDeclaration(DeclMatcher.bind("from_decl"))))),
+          unless(anyOf(hasAncestor(isImplicit()),
+                       hasAncestor(UsingShadowDeclInClass),
+                       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,

Modified: clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp?rev=288919&r1=288918&r2=288919&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp Wed Dec  7 11:04:07 2016
@@ -425,6 +425,36 @@ TEST_F(ChangeNamespaceTest, FixUsingShad
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, DontFixUsingShadowDeclInClasses) {
+  std::string Code = "namespace na {\n"
+                     "class A {};\n"
+                     "class Base { public: Base() {} void m() {} };\n"
+                     "namespace nb {\n"
+                     "class D : public Base {\n"
+                     "public:\n"
+                     "  using AA = A; using B = Base;\n"
+                     "  using Base::m; using Base::Base;\n"
+                     "};"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+
+  std::string Expected = "namespace na {\n"
+                         "class A {};\n"
+                         "class Base { public: Base() {} void m() {} };\n"
+                         "\n"
+                         "} // namespace na\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "class D : public na::Base {\n"
+                         "public:\n"
+                         "  using AA = na::A; using B = na::Base;\n"
+                         "  using Base::m; using Base::Base;\n"
+                         "};"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
 TEST_F(ChangeNamespaceTest, TypeInNestedNameSpecifier) {
   std::string Code =
       "namespace na {\n"