[PATCH] D25309: [clang-move] Support moving multiple classes in one run.

[Eric Liu via cfe-commits]

ioeric added inline comments.


> hokein wrote in ClangMove.cpp:316
> "InMovedClassNames" should not be false in the matcher. Added an assert.

I think you should instead exit early if `ClassNames.empty()` or assert not empty.

> ClangMove.cpp:299
> +  for (StringRef ClassName : ClassNames) {
> +    // Removed the preceding and trailing space of ClassName.
> +    // And removed the global scope operator "::" since we always regard the

The comment is trivial.

> ClangMove.cpp:302
> +    // ClassName as a global name.
> +    llvm::StringRef GlobalClassName = ClassName.trim(' ').ltrim(':');
> +    const auto HasName = hasName(("::" + GlobalClassName).str());

Can we use `trim()` with default argument?

> multiple_class_test.cpp:19
> +}
> +int NoMove::f() {
> +  return 0;

What I wanted to see is another *moved* class because there was no test case where multiple classes are moved into the same namespace.

https://reviews.llvm.org/D25309