[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-11-08 Thread Eric Liu via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL286307: [change-namespace] shorten namespace qualifier based 
on UsingDecl and… (authored by ioeric).

Changed prior to commit:
  https://reviews.llvm.org/D25771?vs=77270&id=77272#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D25771

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

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
@@ -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> InsertFwdDecls;
+  // Records all using declarations, which can be used to shorten namespace
+  // specifiers.
+  llvm::SmallPtrSet UsingDecls;
+  // Records all using namespace declarations, which can be used to shorten
+  // namespace specifiers.
+  llvm::SmallPtrSet UsingNamespaceDecls;
 };
 
 } // namespace change_namespace
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
@@ -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) {
+  SourceLocation DeclLoc = SM.getSpellingLoc(D->getLocation());
+  Loc = SM.getSpellingLoc(Loc);
+  return SM.isBeforeInTranslationUnit(DeclLoc, Loc) &&
+ (SM.getFileID(DeclLoc) == SM.getFileID(Loc) &&
+  isNestedDeclContext(DeclCtx, D->getDeclContext()));
+}
+
 } // anonymous namespace
 
 ChangeNamespaceTool::ChangeNamespaceTool(
@@ -244,17 +262,40 @@
 }
 
 void ChangeNamespaceTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
-  // Match old namespace blocks.
   std::string FullOldNs = "::" + OldNamespace;
+  // Prefix is the outer-most namespace in DiffOldNamespace. For example, if the
+  // OldNamespace is "a::b::c" and DiffOldNamespace is "b::c", then Prefix will
+  // be "a::b". Declarations in this namespace will not be visible in the new
+  // namespace. If DiffOldNamespace is empty, Prefix will be a invalid name "-".
+  llvm::SmallVector DiffOldNsSplitted;
+  llvm::StringRef(DiffOldNamespace).split(DiffOldNsSplitted, "::");
+  std::string Prefix = "-";
+  if (!DiffOldNsSplitted.empty())
+Prefix = (StringRef(FullOldNs).drop_back(DiffOldNamespace.size()) +
+  DiffOldNsSplitted.front())
+ .str();
+  auto IsInMovedNs =
+  allOf(hasAncestor(namespaceDecl(hasName(FullOldNs)).bind("ns_decl")),
+isExpansionInFileMatching(FilePattern));
+  auto IsVisibleInNewNs = anyOf(
+  IsInMovedNs, unless(hasAncestor(namespaceDecl(hasName(Prefix);
+  // Match using declarations.
+  Finder->addMatcher(
+  usingDecl(isExpansionInFileMatching(FilePattern), IsVisibleInNewNs)
+  .bind("using"),
+  this);
+  // Match using namespace declarations.
+  Finder->addMatcher(usingDirectiveDecl(isExpansionInFileMatching(FilePattern),
+IsVisibleInNewNs)
+ .bind("using_namespace"),
+ this);
+
+  // Match old namespace bl

[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-11-08 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 77270.
ioeric marked an inline comment as done.
ioeric added a comment.

- Addressed comment.


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
@@ -313,8 +313,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";
@@ -333,93 +333,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"
@@ -625,6 +546,359 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnC

[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-11-08 Thread Haojian Wu via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM with one nit.




Comment at: change-namespace/ChangeNamespace.cpp:275
+   (DiffOldNsSplitted.empty() ? "-" : DiffOldNsSplitted.front()))
+  .str();
+  auto IsInMovedNs =

ioeric wrote:
> hokein wrote:
> > Using an invalid name `-` is not an elegant solution to me. Is it possible 
> > to avoid it? 
> > Maybe we can explicitly specify `IsVisibleInNewNs` using the code like:
> > 
> > ```
> > Optional> IsVisibleInNewNs = 
> > IsInMovedNs;
> > if (!DiffOldNsSplitted.empty() )  {
> >   std::string Prefix = ... 
> >   IsVisibleInNewNs = anyOf(*IsVisibleInNewNs, 
> > unless(hasAncestor(namespaceDecl(hasName(Prefix));
> > }
> > ```
> As per offline discussion, this seems to be impossible.
OK, then add a comment explicitly specifying that `"-"` is used as an invalid 
name.

I think the code can be simplified as:

```
std::string Prefix = "-";
if (!DiffOldNsSplitted.empty()) {
  Prefix =  (StringRef(FullOldNs).drop_back(DiffOldNamespace.size()) + 
DiffOldNsSplitted.front()).str();
}
```


https://reviews.llvm.org/D25771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-11-08 Thread Eric Liu via cfe-commits
ioeric marked an inline comment as done.
ioeric added inline comments.



Comment at: change-namespace/ChangeNamespace.cpp:275
+   (DiffOldNsSplitted.empty() ? "-" : DiffOldNsSplitted.front()))
+  .str();
+  auto IsInMovedNs =

hokein wrote:
> Using an invalid name `-` is not an elegant solution to me. Is it possible to 
> avoid it? 
> Maybe we can explicitly specify `IsVisibleInNewNs` using the code like:
> 
> ```
> Optional> IsVisibleInNewNs = 
> IsInMovedNs;
> if (!DiffOldNsSplitted.empty() )  {
>   std::string Prefix = ... 
>   IsVisibleInNewNs = anyOf(*IsVisibleInNewNs, 
> unless(hasAncestor(namespaceDecl(hasName(Prefix));
> }
> ```
As per offline discussion, this seems to be impossible.


https://reviews.llvm.org/D25771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-11-08 Thread Haojian Wu via cfe-commits
hokein added a comment.

One more comment, otherwise looks good.




Comment at: change-namespace/ChangeNamespace.cpp:275
+   (DiffOldNsSplitted.empty() ? "-" : DiffOldNsSplitted.front()))
+  .str();
+  auto IsInMovedNs =

Using an invalid name `-` is not an elegant solution to me. Is it possible to 
avoid it? 
Maybe we can explicitly specify `IsVisibleInNewNs` using the code like:

```
Optional> IsVisibleInNewNs = 
IsInMovedNs;
if (!DiffOldNsSplitted.empty() )  {
  std::string Prefix = ... 
  IsVisibleInNewNs = anyOf(*IsVisibleInNewNs, 
unless(hasAncestor(namespaceDecl(hasName(Prefix));
}
```


https://reviews.llvm.org/D25771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-11-08 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 77228.
ioeric marked 2 inline comments as done.
ioeric added a comment.

- Addressed comments.


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
@@ -313,8 +313,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";
@@ -333,93 +333,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"
@@ -625,6 +546,359 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOn

[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-11-08 Thread Haojian Wu via cfe-commits
hokein added inline comments.



Comment at: change-namespace/ChangeNamespace.cpp:233
+ const DeclContext *DeclCtx, SourceLocation Loc) {
+  return SM.isBeforeInTranslationUnit(SM.getSpellingLoc(D->getLocation()),
+  SM.getSpellingLoc(Loc)) &&

I will create two variables for `SM.getSpellingLoc(D->getLocation())` and 
`SM.getSpellingLoc(Loc)` to avoid redundant function call.



Comment at: change-namespace/ChangeNamespace.cpp:569
+StringRef FromDeclNameRef = FromDeclName;
+if (FromDeclNameRef.consume_front(UsingNamespace->getNominatedNamespace()
+  ->getQualifiedNameAsString())) {

Shouldn't we check whether the using namespace decl is visible here? 


https://reviews.llvm.org/D25771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-11-07 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 77115.
ioeric added a comment.

- Get rid of redundant PrefixRef


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
@@ -313,8 +313,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";
@@ -333,93 +333,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"
@@ -625,6 +546,334 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(Chan

[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-11-07 Thread Eric Liu via cfe-commits
ioeric added inline comments.



Comment at: change-namespace/ChangeNamespace.cpp:277
+  auto IsVisibleInOldNs =
+  anyOf(IsInMovedNs, unless(hasAncestor(namespaceDecl(hasName(Prefix);
+  // Match using declarations.

hokein wrote:
> Ignoring using-decls in `Prefix` namespace-decl doesn't work perfectly. The 
> same example:
> 
> ```
> namespace a { void f(); }
> 
> namespace b {
> using a::f;
> namespace c {
> void d() { f(); }
> } 
> }
> ```
> 
> When changing `b::c` to `b::e`, the `using a::f;` will be excluded by this 
> filter. As a result, `a::` will be added to `f()`.
Nice catch! Fixed and explained in the new code. 


https://reviews.llvm.org/D25771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-11-07 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 77114.
ioeric marked 2 inline comments as done.
ioeric added a comment.

- Ignore using decls in old ns but not the inner-most old ns.
- Add a missing test case.
- Addressed comments


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
@@ -313,8 +313,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";
@@ -333,93 +333,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 =
   "nam

[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-11-07 Thread Haojian Wu via cfe-commits
hokein added inline comments.



Comment at: change-namespace/ChangeNamespace.cpp:566
+  break;
+if (isDeclVisibleAtLocation(*Result.SourceManager, Using, DeclCtx, Start)) 
{
+  for (const auto *UsingShadow : Using->shadows()) {

ioeric wrote:
> ioeric wrote:
> > ioeric wrote:
> > > hokein wrote:
> > > > Yeah, it works for most cases, but it can not guarantee that they are 
> > > > still in the same DeclContext after changing to new namespace.
> > > > 
> > > > For example, the following case where an using-decl is used in the 
> > > > namespace being renamed, we change `b::c` to `d::e`, although 
> > > > DeclContext `d` of callexpr `f()` is a nested DeclContext of `b` (also 
> > > > DeclContext of `using a::f`), but this assumption is wrong after 
> > > > changing namespace because we keep `using a::f` in its original 
> > > > namespace.
> > > > 
> > > > ```
> > > > namespace a { void f(); }
> > > > 
> > > > namespace b {
> > > > using a::f;
> > > > namespace c {
> > > > void d() { f(); }
> > > > }  // c
> > > > } // b
> > > > ```
> > > > 
> > > > Not sure whether we should do this in our tool. I suspect there might 
> > > > be a lot of corner cases.
> > > > 
> > > I thought using decls in old namespaces should be moved with along with 
> > > namespaces. If this is the case, the moving of using decls is unexpected 
> > > (looking into this), but this patch is handling this correctly if using 
> > > decls are not moved.
> > Ahh, I was wrong. `using a::f` should not be moved. Hmm, I think we can 
> > solve or at least workaround this. But I still think we should support 
> > shortening namespace specifier based on using decls; it is quite not nice 
> > to add long specifiers if there are already using decls present.
> This should be fixed with the new matcher.
OK, let's try to make it work perfectly.



Comment at: change-namespace/ChangeNamespace.cpp:270
+  auto Pos = StringRef(FullOldNs).rfind(':');
+  // Ignore leading "::".
+  if (Pos != StringRef::npos && Pos > 1)

leading? looks like you are removing trailing  `;`



Comment at: change-namespace/ChangeNamespace.cpp:277
+  auto IsVisibleInOldNs =
+  anyOf(IsInMovedNs, unless(hasAncestor(namespaceDecl(hasName(Prefix);
+  // Match using declarations.

Ignoring using-decls in `Prefix` namespace-decl doesn't work perfectly. The 
same example:

```
namespace a { void f(); }

namespace b {
using a::f;
namespace c {
void d() { f(); }
} 
}
```

When changing `b::c` to `b::e`, the `using a::f;` will be excluded by this 
filter. As a result, `a::` will be added to `f()`.


https://reviews.llvm.org/D25771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-11-07 Thread Eric Liu via cfe-commits
ioeric marked an inline comment as done.
ioeric added a comment.

ping.


https://reviews.llvm.org/D25771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-10-20 Thread Eric Liu via cfe-commits
ioeric marked 3 inline comments as done.
ioeric added inline comments.



Comment at: change-namespace/ChangeNamespace.cpp:566
+  break;
+if (isDeclVisibleAtLocation(*Result.SourceManager, Using, DeclCtx, Start)) 
{
+  for (const auto *UsingShadow : Using->shadows()) {

ioeric wrote:
> ioeric wrote:
> > hokein wrote:
> > > Yeah, it works for most cases, but it can not guarantee that they are 
> > > still in the same DeclContext after changing to new namespace.
> > > 
> > > For example, the following case where an using-decl is used in the 
> > > namespace being renamed, we change `b::c` to `d::e`, although DeclContext 
> > > `d` of callexpr `f()` is a nested DeclContext of `b` (also DeclContext of 
> > > `using a::f`), but this assumption is wrong after changing namespace 
> > > because we keep `using a::f` in its original namespace.
> > > 
> > > ```
> > > namespace a { void f(); }
> > > 
> > > namespace b {
> > > using a::f;
> > > namespace c {
> > > void d() { f(); }
> > > }  // c
> > > } // b
> > > ```
> > > 
> > > Not sure whether we should do this in our tool. I suspect there might be 
> > > a lot of corner cases.
> > > 
> > I thought using decls in old namespaces should be moved with along with 
> > namespaces. If this is the case, the moving of using decls is unexpected 
> > (looking into this), but this patch is handling this correctly if using 
> > decls are not moved.
> Ahh, I was wrong. `using a::f` should not be moved. Hmm, I think we can solve 
> or at least workaround this. But I still think we should support shortening 
> namespace specifier based on using decls; it is quite not nice to add long 
> specifiers if there are already using decls present.
This should be fixed with the new matcher.


https://reviews.llvm.org/D25771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-10-20 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 75281.
ioeric added a comment.

- Add a missing test case.


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,312 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeName

[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-10-20 Thread Eric Liu via cfe-commits
ioeric added a comment.

Still missing one case... working on a fix.


https://reviews.llvm.org/D25771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-10-20 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 75280.
ioeric added a comment.

- Ignore using decls in old ns but not the inner-most old ns.


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,278 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnC

[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-10-20 Thread Eric Liu via cfe-commits
ioeric added inline comments.



Comment at: change-namespace/ChangeNamespace.cpp:566
+  break;
+if (isDeclVisibleAtLocation(*Result.SourceManager, Using, DeclCtx, Start)) 
{
+  for (const auto *UsingShadow : Using->shadows()) {

ioeric wrote:
> hokein wrote:
> > Yeah, it works for most cases, but it can not guarantee that they are still 
> > in the same DeclContext after changing to new namespace.
> > 
> > For example, the following case where an using-decl is used in the 
> > namespace being renamed, we change `b::c` to `d::e`, although DeclContext 
> > `d` of callexpr `f()` is a nested DeclContext of `b` (also DeclContext of 
> > `using a::f`), but this assumption is wrong after changing namespace 
> > because we keep `using a::f` in its original namespace.
> > 
> > ```
> > namespace a { void f(); }
> > 
> > namespace b {
> > using a::f;
> > namespace c {
> > void d() { f(); }
> > }  // c
> > } // b
> > ```
> > 
> > Not sure whether we should do this in our tool. I suspect there might be a 
> > lot of corner cases.
> > 
> I thought using decls in old namespaces should be moved with along with 
> namespaces. If this is the case, the moving of using decls is unexpected 
> (looking into this), but this patch is handling this correctly if using decls 
> are not moved.
Ahh, I was wrong. `using a::f` should not be moved. Hmm, I think we can solve 
or at least workaround this. But I still think we should support shortening 
namespace specifier based on using decls; it is quite not nice to add long 
specifiers if there are already using decls present.


https://reviews.llvm.org/D25771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-10-20 Thread Eric Liu via cfe-commits
ioeric added inline comments.



Comment at: change-namespace/ChangeNamespace.cpp:566
+  break;
+if (isDeclVisibleAtLocation(*Result.SourceManager, Using, DeclCtx, Start)) 
{
+  for (const auto *UsingShadow : Using->shadows()) {

hokein wrote:
> Yeah, it works for most cases, but it can not guarantee that they are still 
> in the same DeclContext after changing to new namespace.
> 
> For example, the following case where an using-decl is used in the namespace 
> being renamed, we change `b::c` to `d::e`, although DeclContext `d` of 
> callexpr `f()` is a nested DeclContext of `b` (also DeclContext of `using 
> a::f`), but this assumption is wrong after changing namespace because we keep 
> `using a::f` in its original namespace.
> 
> ```
> namespace a { void f(); }
> 
> namespace b {
> using a::f;
> namespace c {
> void d() { f(); }
> }  // c
> } // b
> ```
> 
> Not sure whether we should do this in our tool. I suspect there might be a 
> lot of corner cases.
> 
I thought using decls in old namespaces should be moved with along with 
namespaces. If this is the case, the moving of using decls is unexpected 
(looking into this), but this patch is handling this correctly if using decls 
are not moved.


https://reviews.llvm.org/D25771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-10-20 Thread Haojian Wu via cfe-commits
hokein added inline comments.



Comment at: change-namespace/ChangeNamespace.cpp:566
+  break;
+if (isDeclVisibleAtLocation(*Result.SourceManager, Using, DeclCtx, Start)) 
{
+  for (const auto *UsingShadow : Using->shadows()) {

Yeah, it works for most cases, but it can not guarantee that they are still in 
the same DeclContext after changing to new namespace.

For example, the following case where an using-decl is used in the namespace 
being renamed, we change `b::c` to `d::e`, although DeclContext `d` of callexpr 
`f()` is a nested DeclContext of `b` (also DeclContext of `using a::f`), but 
this assumption is wrong after changing namespace because we keep `using a::f` 
in its original namespace.

```
namespace a { void f(); }

namespace b {
using a::f;
namespace c {
void d() { f(); }
}  // c
} // b
```

Not sure whether we should do this in our tool. I suspect there might be a lot 
of corner cases.



https://reviews.llvm.org/D25771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D25771: [change-namespace] shorten namespace qualifier based on UsingDecl and UsingDirectiveDecl.

2016-10-19 Thread Eric Liu via cfe-commits
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));
-}
-
 TES