[cfe-commits] [PATCH] Implements support to run standalone tools

[Marshall Clow]

On Mar 29, 2012, at 4:49 AM, Manuel Klimek wrote:

> Updated patch: now injecting the CompilationDatabase into the
> ClangTool, plus some clean ups.
> 
> Please let me know
> - if that's in line with what you thought

Yes, this is more in line with what I was thinking.

But… ;-)

I think that:

1)
+  /// FIXME: Currently only supports JSON compilation databases, which
+  /// are named 'compile_commands.json' in the given directory. Extend this
+  /// for other build types (like ninja build files).
+  static CompilationDatabase *loadFromDirectory(StringRef BuildDirectory,
+                                                std::string &ErrorMessage);

doesn't belong in CompilationDatabase.
The reason is that in my suggested usage, there will not be a "build directory" to load the compilation database from.
I agree that JSONCompilationDatabase and "MakefileCompilationDatabase" (or whatever) need this, but for simple tools, I'd like to be able to specify all of the options on the command line. 
Things like:
	find . -name \*.cpp -exec tool -option1 -option2 -option3 {} \;

2) runSyntaxOnlyToolOnCode
I don't understand why "input.cc" is hard wired here. 
Is this just some dummy file name - supposed to be unique?
Maybe a comment to say what's going on?

-- Marshall


> - what else I need to do before landing this :)
> 
> Cheers,
> /Manuel
> 
> On Thu, Mar 29, 2012 at 10:53 AM, Manuel Klimek <klimek at google.com> wrote:
>> Updated patch where I pulled out CompilationDatabase.h/.cpp/_test.cpp,
>> to make sure we're all on the same page.
>> 
>> Next step is to dependency inject it into the ClangTool and fix the
>> command line args handling.
>> 
>> Cheers,
>> /Manuel
>> 
>> On Mon, Mar 26, 2012 at 4:43 PM, Douglas Gregor <dgregor at apple.com> wrote:
>>> 
>>> On Mar 26, 2012, at 7:39 AM, Manuel Klimek <klimek at google.com> wrote:
>>> 
>>> On Mon, Mar 26, 2012 at 4:26 PM, Douglas Gregor <dgregor at apple.com> wrote:
>>>> 
>>>> 
>>>> On Mar 26, 2012, at 7:21 AM, Manuel Klimek <klimek at google.com> wrote:
>>>> 
>>>> On Mon, Mar 26, 2012 at 3:56 PM, Douglas Gregor <dgregor at apple.com> wrote:
>>>>> 
>>>>> 
>>>>> On Mar 26, 2012, at 4:08 AM, Manuel Klimek <klimek at google.com> wrote:
>>>>> 
>>>>> 
>>>>>>> Then again, when you say "CompilationDatabase", I think of something
>>>>>>> that I can extract a set { commands, arguments, source file } tuples to be
>>>>>>> executed.
>>>>>>> Maybe that's where we are are talking past each other.
>>>>>> 
>>>>>> When I say CompilationDatabase I think of a map<filename, { command,
>>>>>> arguments, path}>. Does that make sense?
>>>>> 
>>>>> 
>>>>> Makes sense to me, although of course this could end up being a multimap.
>>>> 
>>>> 
>>>> Which is also an interesting design question - at least for refactorings
>>>> we'll probably want to run over all different command lines the file was
>>>> compiled with (think: different configurations with different macros
>>>> enabled). This would lead to a slightly different interface for
>>>> CompilationDatabase though (returning a list of compile command lines for
>>>> each target).
>>>> 
>>>> 
>>>> I think it's important for us to support this case in the interface.
>>> 
>>> 
>>> Agreed. So something like:
>>> 
>>> /// \brief Specifies the working directory and command of a compilation.
>>> struct CompileCommand {
>>>   /// \brief The working directory the command was executed from.
>>>   std::string Directory;
>>> 
>>>   /// \brief The command line that was executed.
>>>   std::vector<std::string> CommandLine;
>>> };
>>> 
>>> class CompilationDatabase {
>>> public:
>>>   /// \brief Returns all compile commands in which the specified file was
>>> compiled.
>>>   ///
>>>   /// This includes compile commands that span multiple source files.
>>>   /// For example, for a compile command line
>>>   /// $ clang++ -o test a.cc b.cc t.cc
>>>   /// $ clang++ -o production a.cc b.cc -DPRODUCTION
>>>   /// A compilation database representing the project would return both
>>> command lines
>>>   /// for a.cc and b.cc and only the first command line for t.cc.
>>>   virtual vector<CompileCommand> getCompileCommands(StringRef FilePath) = 0;
>>> };
>>> 
>>> 
>>> Sure. Returning vectors is cheap now! ;)
>>> 
>>> Also, I'd like to pull CompilationDatabase into its own header file if that
>>> is fine with you.
>>> 
>>> 
>>> Yes, please.
>>> 
>>> - Doug
>>> 
> <tooling.patch>

-- Marshall

Marshall Clow     Idio Software   <mailto:mclow.lists at gmail.com>

A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait).
        -- Yu Suzuki