[PATCH] D28052: [change-namespace] consider namespace aliases to shorten qualified names.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 22 07:45:32 PST 2016


hokein added inline comments.


================
Comment at: change-namespace/ChangeNamespace.cpp:719
+        // The alias is defined in some namespace.
+        assert(AliasQualifiedName.size() >= AliasName.size() + 2);
+        llvm::StringRef AliasNs =
----------------
maybe `assert(StringRef(AliasQualifiedName).end_with("::"+AliasName))`?


================
Comment at: change-namespace/ChangeNamespace.cpp:741
         if (TargetDecl == FromDecl) {
           ReplaceName = FromDecl->getNameAsString();
           Matched = true;
----------------
Do we also need to check  `FromDecl->getNameAsString().size() < ReplaceName.size()` before assigning the name to `ReplaceName`?


================
Comment at: unittests/change-namespace/ChangeNamespaceTests.cpp:832
+                     "}\n"
+                     "namespace gl = glob;\n"
+                     "namespace na {\n"
----------------
Add a testcase for the namespace alias decl with preceding `::`,  e.g. `namespace gl = ::glob`?


https://reviews.llvm.org/D28052





More information about the cfe-commits mailing list