ioeric added inline comments.
> ClangMove.cpp:295
> void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
> - std::string FullyQualifiedName = "::" + Spec.Name;
> + auto ParseNames = [](llvm::StringRef Names) {
> + SmallVector<StringRef, 4> ClassNames;
Why create this lambda if it's only used once? I think you can save a few lines
by just inlining this.
> ClangMove.cpp:303
> + for (StringRef ClassName : ClassNames) {
> + const auto HasName = hasName(("::" + ClassName.ltrim(":")).str());
> + if (InMovedClassNames)
I'd also allow whitespaces around separators, e.g. `a::b::X, a::b::Y`. Maybe
also `trim()` the name.
> ClangMove.cpp:304
> + const auto HasName = hasName(("::" + ClassName.ltrim(":")).str());
> + if (InMovedClassNames)
> + InMovedClassNames = anyOf(*InMovedClassNames, HasName);
Does this work?
InMovedClassNames = InMovedClassNames ? ... : ...
> ClangMove.cpp:316
> auto InMovedClass =
> - hasDeclContext(cxxRecordDecl(hasName(FullyQualifiedName)));
> + hasDeclContext(cxxRecordDecl(*InMovedClassNames));
>
What would happen if `InMovedClassNames == false`?
> ClangMove.h:40
> struct MoveDefinitionSpec {
> - // A fully qualified name, e.g. "X", "a::X".
> - std::string Name;
> + // A semicolon-separated list of fully qualified names, e.g. "Foo",
> + // "a::Foo;b::Foo".
I'd use commas as separator. Eyeballs can easily mix ";" with ":".
> ClangMoveMain.cpp:41
> +cl::opt<std::string>
> + Name("name", cl::desc("A semicolon-separated list of the names of
> classes "
> + "being moved, e.g. \"Foo\", \"a::Foo;b::Foo\"."),
Same here if semi->comma.
And I'd name this `Names` and `names`
> multiple_class_test.cpp:15
> +
> +namespace c {
> +int Bar::f() {
Can you add one more moved class that is also in `namespace c`?
https://reviews.llvm.org/D25309
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
