[PATCH] D26456: Handle adding new nested namespace in old namespace.

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 10 10:28:26 PST 2016


hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM with two nits.



================
Comment at: change-namespace/ChangeNamespace.cpp:475
+  // If there is no outer namespace (i.e. DiffOldNamespace is empty), the new
+  // namespace will be a nested namespace in the old namespace
   const NamespaceDecl *OuterNs = getOuterNamespace(NsDecl, DiffOldNamespace);
----------------
missing a trailing `.`.


================
Comment at: change-namespace/ChangeNamespace.cpp:477
   const NamespaceDecl *OuterNs = getOuterNamespace(NsDecl, DiffOldNamespace);
-  SourceLocation LocAfterNs =
-      getStartOfNextLine(OuterNs->getRBraceLoc(), *Result.SourceManager,
-                         Result.Context->getLangOpts());
-  assert(LocAfterNs.isValid() &&
-         "Failed to get location after DiffOldNamespace");
+  SourceLocation InsertionLoc;
+  if (OuterNs) {
----------------
How about `SourceLocation InsertionLoc = Start;` so that you don't need else statement below?


https://reviews.llvm.org/D26456





More information about the cfe-commits mailing list