[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
Tue Nov 8 09:54:20 PST 2016


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

Lg with a few nits.



================
Comment at: clang-move/ClangMove.cpp:539
+  const FileEntry *FE = SM.getFileManager().getFile(
+      MakeAbsolutePath(OriginalRunningDirectory, OldFile));
+  if (!FE) {
----------------
I'd probably pull this out as an inline function to save some typing.

```
    inline std::string ClangMoveTool::MakeAbsolute(Path) {
        return MakeAbsolutePath(OriginalRunningDirectory, OldFile));
    }
```




================
Comment at: unittests/clang-move/ClangMoveTests.cpp:286
+    auto Results = runClangMoveOnCode(Spec, Header.c_str(), Code);
+    EXPECT_EQ(Header, Results[Spec.NewHeader]);
+  }
----------------
Please also check old header is removed or empty.


================
Comment at: unittests/clang-move/ClangMoveTests.cpp:329
+    auto Results = runClangMoveOnCode(Spec, Header.c_str(), Code);
+    EXPECT_EQ(ExpectedHeader, Results[Spec.NewHeader]);
+  }
----------------
Please also check old headers.


https://reviews.llvm.org/D26236





More information about the cfe-commits mailing list