<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Aug 27, 2014 at 2:36 PM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: alexfh<br>
Date: Wed Aug 27 16:36:39 2014<br>
New Revision: 216620<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=216620&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=216620&view=rev</a><br>
Log:<br>
Query CompilationDatabase right before running each compilation.<br>
<br>
Summary:<br>
Query CompilationDatabase right before running each compilation. This allows<br>
supporting compilation databases that change external state required for<br>
successful compilation.<br></blockquote><div><br></div><div>Could you give an example of this? It's not clear to me what this means.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Reviewers: klimek, djasper<br>
<br>
Reviewed By: djasper<br>
<br>
Subscribers: klimek, cfe-commits<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D5086" target="_blank">http://reviews.llvm.org/D5086</a><br>
<br>
Modified:<br>
cfe/trunk/include/clang/Tooling/Tooling.h<br>
cfe/trunk/lib/Tooling/Tooling.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Tooling/Tooling.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Tooling.h?rev=216620&r1=216619&r2=216620&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Tooling.h?rev=216620&r1=216619&r2=216620&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Tooling/Tooling.h (original)<br>
+++ cfe/trunk/include/clang/Tooling/Tooling.h Wed Aug 27 16:36:39 2014<br>
@@ -293,8 +293,8 @@ class ClangTool {<br>
FileManager &getFiles() { return *Files; }<br>
<br>
private:<br>
- // We store compile commands as pair (file name, compile command).<br>
- std::vector< std::pair<std::string, CompileCommand> > CompileCommands;<br>
+ const CompilationDatabase &Compilations;<br>
+ std::vector<std::string> SourcePaths;<br>
<br>
llvm::IntrusiveRefCntPtr<FileManager> Files;<br>
// Contains a list of pairs (<file name>, <file content>).<br>
<br>
Modified: cfe/trunk/lib/Tooling/Tooling.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=216620&r1=216619&r2=216620&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=216620&r1=216619&r2=216620&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Tooling/Tooling.cpp (original)<br>
+++ cfe/trunk/lib/Tooling/Tooling.cpp Wed Aug 27 16:36:39 2014<br>
@@ -273,28 +273,10 @@ bool FrontendActionFactory::runInvocatio<br>
<br>
ClangTool::ClangTool(const CompilationDatabase &Compilations,<br>
ArrayRef<std::string> SourcePaths)<br>
- : Files(new FileManager(FileSystemOptions())), DiagConsumer(nullptr) {<br>
+ : Compilations(Compilations), SourcePaths(SourcePaths),<br>
+ Files(new FileManager(FileSystemOptions())), DiagConsumer(nullptr) {<br>
ArgsAdjusters.push_back(new ClangStripOutputAdjuster());<br>
ArgsAdjusters.push_back(new ClangSyntaxOnlyAdjuster());<br>
- for (const auto &SourcePath : SourcePaths) {<br>
- std::string File(getAbsolutePath(SourcePath));<br>
-<br>
- std::vector<CompileCommand> CompileCommandsForFile =<br>
- Compilations.getCompileCommands(File);<br>
- if (!CompileCommandsForFile.empty()) {<br>
- for (CompileCommand &CompileCommand : CompileCommandsForFile) {<br>
- CompileCommands.push_back(<br>
- std::make_pair(File, std::move(CompileCommand)));<br>
- }<br>
- } else {<br>
- // FIXME: There are two use cases here: doing a fuzzy<br>
- // "find . -name '*.cc' |xargs tool" match, where as a user I don't care<br>
- // about the .cc files that were not found, and the use case where I<br>
- // specify all files I want to run over explicitly, where this should<br>
- // be an error. We'll want to add an option for this.<br>
- llvm::errs() << "Skipping " << File << ". Compile command not found.\n";<br>
- }<br>
- }<br>
}<br>
<br>
void ClangTool::setDiagnosticConsumer(DiagnosticConsumer *D) {<br>
@@ -333,36 +315,49 @@ int ClangTool::run(ToolAction *Action) {<br>
llvm::sys::fs::getMainExecutable("clang_tool", &StaticSymbol);<br>
<br>
bool ProcessingFailed = false;<br>
- for (const auto &Command : CompileCommands) {<br>
- // FIXME: chdir is thread hostile; on the other hand, creating the same<br>
- // behavior as chdir is complex: chdir resolves the path once, thus<br>
- // guaranteeing that all subsequent relative path operations work<br>
- // on the same path the original chdir resulted in. This makes a difference<br>
- // for example on network filesystems, where symlinks might be switched<br>
- // during runtime of the tool. Fixing this depends on having a file system<br>
- // abstraction that allows openat() style interactions.<br>
- if (chdir(Command.second.Directory.c_str()))<br>
- llvm::report_fatal_error("Cannot chdir into \"" +<br>
- Twine(Command.second.Directory) + "\n!");<br>
- std::vector<std::string> CommandLine = Command.second.CommandLine;<br>
- for (ArgumentsAdjuster *Adjuster : ArgsAdjusters)<br>
- CommandLine = Adjuster->Adjust(CommandLine);<br>
- assert(!CommandLine.empty());<br>
- CommandLine[0] = MainExecutable;<br>
- // FIXME: We need a callback mechanism for the tool writer to output a<br>
- // customized message for each file.<br>
- DEBUG({<br>
- llvm::dbgs() << "Processing: " << Command.first << ".\n";<br>
- });<br>
- ToolInvocation Invocation(std::move(CommandLine), Action, Files.get());<br>
- Invocation.setDiagnosticConsumer(DiagConsumer);<br>
- for (const auto &MappedFile : MappedFileContents) {<br>
- Invocation.mapVirtualFile(MappedFile.first, MappedFile.second);<br>
+ for (const auto &SourcePath : SourcePaths) {<br>
+ std::string File(getAbsolutePath(SourcePath));<br>
+<br>
+ std::vector<CompileCommand> CompileCommandsForFile =<br>
+ Compilations.getCompileCommands(File);<br></blockquote><div><br></div><div>The analysis that I did in <a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-September/032234.html">http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-September/032234.html</a> still holds AFAIK, so this line here takes O(project size) time to execute for a JSON compilation database. The containing loop has O(project size) iterations when running a refactoring tool over a codebase, which means that the performance of this routine is now O(N^2), no? That seems unacceptable for our "default" suggested way for people to use clang tooling (JSON compilation database + single tool).</div>
<div><br></div><div><div>-- Sean Silva</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ if (CompileCommandsForFile.empty()) {<br>
+ // FIXME: There are two use cases here: doing a fuzzy<br>
+ // "find . -name '*.cc' |xargs tool" match, where as a user I don't care<br>
+ // about the .cc files that were not found, and the use case where I<br>
+ // specify all files I want to run over explicitly, where this should<br>
+ // be an error. We'll want to add an option for this.<br>
+ llvm::errs() << "Skipping " << File << ". Compile command not found.\n";<br>
+ continue;<br>
}<br>
- if (!Invocation.run()) {<br>
- // FIXME: Diagnostics should be used instead.<br>
- llvm::errs() << "Error while processing " << Command.first << ".\n";<br>
- ProcessingFailed = true;<br>
+ for (CompileCommand &CompileCommand : CompileCommandsForFile) {<br>
+ // FIXME: chdir is thread hostile; on the other hand, creating the same<br>
+ // behavior as chdir is complex: chdir resolves the path once, thus<br>
+ // guaranteeing that all subsequent relative path operations work<br>
+ // on the same path the original chdir resulted in. This makes a<br>
+ // difference for example on network filesystems, where symlinks might be<br>
+ // switched during runtime of the tool. Fixing this depends on having a<br>
+ // file system abstraction that allows openat() style interactions.<br>
+ if (chdir(CompileCommand.Directory.c_str()))<br>
+ llvm::report_fatal_error("Cannot chdir into \"" +<br>
+ Twine(CompileCommand.Directory) + "\n!");<br>
+ std::vector<std::string> CommandLine = CompileCommand.CommandLine;<br>
+ for (ArgumentsAdjuster *Adjuster : ArgsAdjusters)<br>
+ CommandLine = Adjuster->Adjust(CommandLine);<br>
+ assert(!CommandLine.empty());<br>
+ CommandLine[0] = MainExecutable;<br>
+ // FIXME: We need a callback mechanism for the tool writer to output a<br>
+ // customized message for each file.<br>
+ DEBUG({ llvm::dbgs() << "Processing: " << File << ".\n"; });<br>
+ ToolInvocation Invocation(std::move(CommandLine), Action, Files.get());<br>
+ Invocation.setDiagnosticConsumer(DiagConsumer);<br>
+ for (const auto &MappedFile : MappedFileContents) {<br>
+ Invocation.mapVirtualFile(MappedFile.first, MappedFile.second);<br>
+ }<br>
+ if (!Invocation.run()) {<br>
+ // FIXME: Diagnostics should be used instead.<br>
+ llvm::errs() << "Error while processing " << File << ".\n";<br>
+ ProcessingFailed = true;<br>
+ }<br>
}<br>
}<br>
return ProcessingFailed ? 1 : 0;<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>