================
Comment at: include/clang/Tooling/Tooling.h:79
@@ -63,1 +78,3 @@
+ bool runInvocation(clang::CompilerInvocation *Invocation,
+ FileManager *Files);
----------------
Does doxygen default to having the base class' comment here?
================
Comment at: include/clang/Tooling/Tooling.h:275
@@ -225,1 +274,3 @@
+ /// append them to ASTs.
+ int buildASTs(std::vector<ASTUnit *> &ASTs);
----------------
Hm. ClangTool is made to be extended, for example by RefactoringClangTool. I'm
unsure how buildAST fits that interface. I've not read the rest of the patch
yet, but it's unclear to me how buildAST here is related to run() from the
comment. If it's not, why is it in ClangTool? If it is, I think we need to
explain it here...
================
Comment at: lib/Tooling/Tooling.cpp:118
@@ +117,3 @@
+ Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
+ Args.push_back(FileName.data());
+ return Args;
----------------
.str()? I don't think .data() is always 0-terminated.
================
Comment at: lib/Tooling/Tooling.cpp:130
@@ -119,1 +129,3 @@
+ ToolInvocation Invocation(getSyntaxOnlyToolArgs(Args, FileNameRef),
ToolAction,
+ &*Files);
----------------
I'd use getPtr().
================
Comment at: lib/Tooling/Tooling.cpp:159
@@ +158,3 @@
+public:
+ SingleFrontendActionFactory(FrontendAction *FAction) : FAction(FAction) {}
+
----------------
Can we %s/FAction/Action/? I don't think the F gives me anything.
================
Comment at: lib/Tooling/Tooling.cpp:174
@@ +173,3 @@
+ : CommandLine(CommandLine.vec()),
+ Action(new SingleFrontendActionFactory(FAction)), OwnsAction(true),
+ Files(Files) {}
----------------
I'm usually opposed to Owns* patterns, but I don't see a better way here (for
now) without changing all the callers...
================
Comment at: lib/Tooling/Tooling.cpp:198-200
@@ -197,5 +243,1 @@
- // ToolAction can have lifetime requirements for Compiler or its members, and
- // we need to ensure it's deleted earlier than Compiler. So we pass it to an
- // OwningPtr declared after the Compiler variable.
- OwningPtr<FrontendAction> ScopedToolAction(ToolAction.take());
----------------
Did you delete this comment intentionally?
================
Comment at: lib/Tooling/Tooling.cpp:339
@@ -311,3 +338,3 @@
});
- ToolInvocation Invocation(CommandLine, ActionFactory->create(), &Files);
+ ToolInvocation Invocation(CommandLine, Action, &*Files);
for (int I = 0, E = MappedFileContents.size(); I != E; ++I) {
----------------
I'd use getPtr().
================
Comment at: lib/Tooling/Tooling.cpp:209
@@ -208,3 +251,2 @@
Compiler.createSourceManager(*Files);
- addFileMappingsTo(Compiler.getSourceManager());
----------------
Huh? Why can we delete this?
http://llvm-reviews.chandlerc.com/D2097
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
