[PATCH] D26966: [clang-move] Add some options allowing to add old/new.h to new/old.h respectively.

[Haojian Wu via cfe-commits]

hokein added inline comments.


================
Comment at: clang-move/ClangMove.cpp:309
+                           llvm::StringRef FileName, bool IsHeader = false,
+                           StringRef OldHeaderInclude = "") {
   std::string NewCode;
----------------
ioeric wrote:
> How do you handle the case where a whole file is copied?
Cases where a whole file is moved are handled in other functions, not in this function.


================
Comment at: clang-move/ClangMove.cpp:416
+void ClangMoveTool::addRemovedDecl(const MovedDecl &Decl) {
+  const auto &SM = *Decl.SM;
+  auto Loc = Decl.Decl->getLocation();
----------------
ioeric wrote:
> This function is not very straight-forward to me so need some comments on what and why.
Added a description in the method declaration.


================
Comment at: clang-move/ClangMove.cpp:613
+          MakeAbsolutePath(*SM, FilePath) == makeAbsolutePath(Spec.OldHeader)) {
+        std::string IncludeNewH = "#include \""  + Spec.NewHeader + "\"\n";
+        auto Err = It.second.add(
----------------
ioeric wrote:
> We might want to do what include-fixer does to compute the best path to put in #include, e.g. we'd want `#include "llvm/XXX.h"` instead of `#include "llvm/include/llvm/XXX.h"`
It might make sense, but it will require non-trivial change of this patch. Add a FIXME.


https://reviews.llvm.org/D26966