[PATCH] D26236: [clang-move] Move all code from old.h/cc directly when moving all class declarations from old.h.

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 7 14:24:31 PST 2016


ioeric added inline comments.


================
Comment at: unittests/clang-move/ClangMoveTests.cpp:278
+  const char Code[] = "#include \"foo.h\"\nint A::f() { return 0; }";
+  Spec.Names = {std::string("A")};
+  Spec.OldHeader = "foo.h";
----------------
hokein wrote:
> ioeric wrote:
> > Maybe just insert or push back?
> I think using `insert` or `push_back` doesn't make the code as clear as initialization here. Code readers might pull up the source file to find the code for the initialized value of this variable.
But you have only one element here right? With those, you wouldn't need std::string(...) I guess.


================
Comment at: unittests/clang-move/ClangMoveTests.cpp:299
+    "using Int=int;\nclass A {\npublic:\n  int f();\n};",
+    "namespace a {}\nusing namespace a;\nclass A {\npublic:\n  int f();\n};",
+    "class B {};\nclass A {\npublic:\n  int f();\n};",
----------------
hokein wrote:
> ioeric wrote:
> > I think using namespace decl should also be ignored?
> This case should rarely happen in source code, as many code styles prohibit `using-namespace` decls in headers.
> I'd like to keep the current behavior because the using-namespace decl will populate the namespace in all the files which include this header. Ignoring it might break a lot of code.
But if you are moving the whole file, using namespace would also be moved so that code would not break?


https://reviews.llvm.org/D26236





More information about the cfe-commits mailing list