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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
