r216620 - Query CompilationDatabase right before running each compilation.
Alexander Kornienko
alexfh at google.com
Wed Aug 27 16:28:15 PDT 2014
On Thu, Aug 28, 2014 at 1:03 AM, Sean Silva <chisophugis at gmail.com> wrote:
> On Wed, Aug 27, 2014 at 2:36 PM, Alexander Kornienko <alexfh at google.com>
> wrote:
>
>> Author: alexfh
>> Date: Wed Aug 27 16:36:39 2014
>> New Revision: 216620
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=216620&view=rev
>> Log:
>> Query CompilationDatabase right before running each compilation.
>>
>> Summary:
>> Query CompilationDatabase right before running each compilation. This
>> allows
>> supporting compilation databases that change external state required for
>> successful compilation.
>>
>
> Could you give an example of this? It's not clear to me what this means.
>
This means, that in some implementations the call to
Compilations.getCompileCommands(File) may make changes in the file system
to allow compilation of the File (e.g. generate headers from .td files).
State of the file system required for compiling one file may be
incompatible with the state required for compiling another file, so we
actually need to run the tool on the file right after we call
getCompileCommands for this file.
>
>
>> Reviewers: klimek, djasper
>>
>> Reviewed By: djasper
>>
>> Subscribers: klimek, cfe-commits
>>
>> Differential Revision: http://reviews.llvm.org/D5086
>>
>> Modified:
>> cfe/trunk/include/clang/Tooling/Tooling.h
>> cfe/trunk/lib/Tooling/Tooling.cpp
>>
>> Modified: cfe/trunk/include/clang/Tooling/Tooling.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Tooling.h?rev=216620&r1=216619&r2=216620&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Tooling/Tooling.h (original)
>> +++ cfe/trunk/include/clang/Tooling/Tooling.h Wed Aug 27 16:36:39 2014
>> @@ -293,8 +293,8 @@ class ClangTool {
>> FileManager &getFiles() { return *Files; }
>>
>> private:
>> - // We store compile commands as pair (file name, compile command).
>> - std::vector< std::pair<std::string, CompileCommand> > CompileCommands;
>> + const CompilationDatabase &Compilations;
>> + std::vector<std::string> SourcePaths;
>>
>> llvm::IntrusiveRefCntPtr<FileManager> Files;
>> // Contains a list of pairs (<file name>, <file content>).
>>
>> Modified: cfe/trunk/lib/Tooling/Tooling.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=216620&r1=216619&r2=216620&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Tooling/Tooling.cpp (original)
>> +++ cfe/trunk/lib/Tooling/Tooling.cpp Wed Aug 27 16:36:39 2014
>> @@ -273,28 +273,10 @@ bool FrontendActionFactory::runInvocatio
>>
>> ClangTool::ClangTool(const CompilationDatabase &Compilations,
>> ArrayRef<std::string> SourcePaths)
>> - : Files(new FileManager(FileSystemOptions())), DiagConsumer(nullptr)
>> {
>> + : Compilations(Compilations), SourcePaths(SourcePaths),
>> + Files(new FileManager(FileSystemOptions())), DiagConsumer(nullptr)
>> {
>> ArgsAdjusters.push_back(new ClangStripOutputAdjuster());
>> ArgsAdjusters.push_back(new ClangSyntaxOnlyAdjuster());
>> - for (const auto &SourcePath : SourcePaths) {
>> - std::string File(getAbsolutePath(SourcePath));
>> -
>> - std::vector<CompileCommand> CompileCommandsForFile =
>> - Compilations.getCompileCommands(File);
>> - if (!CompileCommandsForFile.empty()) {
>> - for (CompileCommand &CompileCommand : CompileCommandsForFile) {
>> - CompileCommands.push_back(
>> - std::make_pair(File, std::move(CompileCommand)));
>> - }
>> - } else {
>> - // FIXME: There are two use cases here: doing a fuzzy
>> - // "find . -name '*.cc' |xargs tool" match, where as a user I
>> don't care
>> - // about the .cc files that were not found, and the use case where
>> I
>> - // specify all files I want to run over explicitly, where this
>> should
>> - // be an error. We'll want to add an option for this.
>> - llvm::errs() << "Skipping " << File << ". Compile command not
>> found.\n";
>> - }
>> - }
>> }
>>
>> void ClangTool::setDiagnosticConsumer(DiagnosticConsumer *D) {
>> @@ -333,36 +315,49 @@ int ClangTool::run(ToolAction *Action) {
>> llvm::sys::fs::getMainExecutable("clang_tool", &StaticSymbol);
>>
>> bool ProcessingFailed = false;
>> - for (const auto &Command : CompileCommands) {
>> - // FIXME: chdir is thread hostile; on the other hand, creating the
>> same
>> - // behavior as chdir is complex: chdir resolves the path once, thus
>> - // guaranteeing that all subsequent relative path operations work
>> - // on the same path the original chdir resulted in. This makes a
>> difference
>> - // for example on network filesystems, where symlinks might be
>> switched
>> - // during runtime of the tool. Fixing this depends on having a file
>> system
>> - // abstraction that allows openat() style interactions.
>> - if (chdir(Command.second.Directory.c_str()))
>> - llvm::report_fatal_error("Cannot chdir into \"" +
>> - Twine(Command.second.Directory) + "\n!");
>> - std::vector<std::string> CommandLine = Command.second.CommandLine;
>> - for (ArgumentsAdjuster *Adjuster : ArgsAdjusters)
>> - CommandLine = Adjuster->Adjust(CommandLine);
>> - assert(!CommandLine.empty());
>> - CommandLine[0] = MainExecutable;
>> - // FIXME: We need a callback mechanism for the tool writer to output
>> a
>> - // customized message for each file.
>> - DEBUG({
>> - llvm::dbgs() << "Processing: " << Command.first << ".\n";
>> - });
>> - ToolInvocation Invocation(std::move(CommandLine), Action,
>> Files.get());
>> - Invocation.setDiagnosticConsumer(DiagConsumer);
>> - for (const auto &MappedFile : MappedFileContents) {
>> - Invocation.mapVirtualFile(MappedFile.first, MappedFile.second);
>> + for (const auto &SourcePath : SourcePaths) {
>> + std::string File(getAbsolutePath(SourcePath));
>> +
>> + std::vector<CompileCommand> CompileCommandsForFile =
>> + Compilations.getCompileCommands(File);
>>
>
> The analysis that I did in
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-September/032234.html still
> holds AFAIK, so this line here takes O(project size) time to execute for a
> JSON compilation database.
>
Does it? JSONCompilationDatabase stores commands in llvm::StringMap indexed
by filename. I would be surprised, if it took O(N) to fetch an item.
> 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).
>
> -- Sean Silva
>
>
>> + if (CompileCommandsForFile.empty()) {
>> + // FIXME: There are two use cases here: doing a fuzzy
>> + // "find . -name '*.cc' |xargs tool" match, where as a user I
>> don't care
>> + // about the .cc files that were not found, and the use case where
>> I
>> + // specify all files I want to run over explicitly, where this
>> should
>> + // be an error. We'll want to add an option for this.
>> + llvm::errs() << "Skipping " << File << ". Compile command not
>> found.\n";
>> + continue;
>> }
>> - if (!Invocation.run()) {
>> - // FIXME: Diagnostics should be used instead.
>> - llvm::errs() << "Error while processing " << Command.first <<
>> ".\n";
>> - ProcessingFailed = true;
>> + for (CompileCommand &CompileCommand : CompileCommandsForFile) {
>> + // FIXME: chdir is thread hostile; on the other hand, creating the
>> same
>> + // behavior as chdir is complex: chdir resolves the path once, thus
>> + // guaranteeing that all subsequent relative path operations work
>> + // on the same path the original chdir resulted in. This makes a
>> + // difference for example on network filesystems, where symlinks
>> might be
>> + // switched during runtime of the tool. Fixing this depends on
>> having a
>> + // file system abstraction that allows openat() style interactions.
>> + if (chdir(CompileCommand.Directory.c_str()))
>> + llvm::report_fatal_error("Cannot chdir into \"" +
>> + Twine(CompileCommand.Directory) +
>> "\n!");
>> + std::vector<std::string> CommandLine = CompileCommand.CommandLine;
>> + for (ArgumentsAdjuster *Adjuster : ArgsAdjusters)
>> + CommandLine = Adjuster->Adjust(CommandLine);
>> + assert(!CommandLine.empty());
>> + CommandLine[0] = MainExecutable;
>> + // FIXME: We need a callback mechanism for the tool writer to
>> output a
>> + // customized message for each file.
>> + DEBUG({ llvm::dbgs() << "Processing: " << File << ".\n"; });
>> + ToolInvocation Invocation(std::move(CommandLine), Action,
>> Files.get());
>> + Invocation.setDiagnosticConsumer(DiagConsumer);
>> + for (const auto &MappedFile : MappedFileContents) {
>> + Invocation.mapVirtualFile(MappedFile.first, MappedFile.second);
>> + }
>> + if (!Invocation.run()) {
>> + // FIXME: Diagnostics should be used instead.
>> + llvm::errs() << "Error while processing " << File << ".\n";
>> + ProcessingFailed = true;
>> + }
>> }
>> }
>> return ProcessingFailed ? 1 : 0;
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140828/660edeff/attachment.html>
More information about the cfe-commits
mailing list