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

[Manuel Klimek]

On Wed, Mar 21, 2012 at 10:44 PM, Marshall Clow <mclow.lists at gmail.com> wrote:
> On Mar 21, 2012, at 8:07 AM, Manuel Klimek wrote:
>> On Wed, Mar 21, 2012 at 3:09 PM, Marshall Clow <mclow.lists at gmail.com> wrote:
>>> On Mar 21, 2012, at 6:25 AM, Manuel Klimek wrote:
>>>
>>>> So, I've looked through the design ideas and I have a few follow up
>>>> questions / remarks.
>>>>
>>>> On Wed, Mar 14, 2012 at 1:35 PM, Douglas Gregor <dgregor at apple.com> wrote:
>>>>> I suggest a more general architecture that separates out the "program build" database from the code that parses code and runs the action on that code. To be a bit more concrete, I'm suggesting splitting ClangTool into two subsystems:
>>>>
>>>> Just to reiterate: I think this is a great idea.
>>>>
>>>>>        - An abstract class CompilationDatabase that allows one to iterate over the CompileCommands that went into a particular build (or apply some function object to each CompileCommand), along with:
>>>>>                - A subclass JSONCompilationDatabase that retrieves CompileCommands from the JSON output produced by CMake
>>>>>                - A subclass SimpleCompilationDatabase where one programmatically adds CompileCommands
>>>>>                - Note: it should be possible for us to create additional compilation database kinds, e.g., by parsing makefiles directly
>>>>>                - A function that makes it easy to take argc/argv an interpret it either as a use of JSONCompilationDatabase or SimpleCompilationDatabase. This should make it easy to create a unified command-line interface to Tooling-based Clang tools, so it's easy for end users to run different tools on their projects
>>>>
>>>> I'm not sure why we'd need argc/argv here. I'd propose a function like
>>>> (stealing from what you wrote further down, slightly changing it):
>>>> /// \brief Loads a compilation database from a build directory.
>>>> ///
>>>> /// Looks at the specified 'BuildDirectory' and creates a compilation
>>>> database that allows to query compile commands for source files in the
>>>> corresponding source tree.
>>>> /// Returns NULL if we were not able to build up a compilation
>>>> database for the build directory.
>>>> /// FIXME: Currently only supports JSON compilation databases, which
>>>> are named 'compile_commands.json' in the given directory.
>>>> CompilationDatabase *loadCompilationDatabase(StringRef BuildDirectory);
>>>
>>> I think that Doug's point here is that sometimes you want to get your options/list of files from either a Makefile or argc/argv, rather than a JSON file, and putting a layer of abstraction here ("compilation database"), lets you do that.
>>
>> I agree with that sentence on all levels, and that was also my
>> impression of Doug's point. I'm not sure I see why you think I
>> disagree, so let me try to rephrase / clarify what exactly the point
>> is that I think we want to do differently from what Doug wrote.
>
> OK.
>
>> I'm mainly proposing that we don't use the (argc, argv) abstraction
>> for specifying the location of whatever we build our compilation
>> database from, as that limits the way tools can use arguments (or it
>> gets really ugly to construct those c-structures).
>
> Sounds good to me.
>
>> I'm thus making a counter-proposal: using the notion of a build
>> directory to specify where we want to get our compile commands for.
>> This is a factory method that will be extended to look at the
>> directory and figure out what is there to create the compile commands
>> for. For example, it can look for a compile_commands.json file and
>> build a JSONCompilationDatabase from that. Or it can see Makefiles
>> that are parsable enough and build a MakefileCompilationDatabase. Or
>> it finds ninja files and creates a NinjaCompilationDatabase.
>>
>> If the build directory is a bad idea (which is might well be), I'd be
>> interested in different proposals, or an argument of why (argc,argv)
>> is what we want to create the compilation database from.
>
> I think I understand now.  The point that I was trying to make is that I think that for simple tools we (or at least I) would like to be able to forego building some kind of database (JSON, Makefile, Ninja), and just specify the options and source files on the command line.
>
> $ ./oneOffTool -I blah -Dblix=4 -I other_dir -o output_dir -someCustomFlag *.cpp
>
> As long as we're abstracting the idea of a "compilation database", I think that generating such a db from an argc/argv pair seems like a good idea (and not very hard).
>
> Hopefully what I'm trying to convey is clearer now.

Well, the interesting thing is how to design this. From a design point
of view, the command line becomes very different depending on whether
you basically want to "forward all args to clang in the tool" or "find
the args for the specified files and run the clang tool over them".
The first one inherently means I can only specify the command line for
a single invocation, so it is also pretty limited. I agree it is an
important use case though.

A possible solution would be to allow for a command line flag that
switches to use the command line arguments directly as arguments for
the clang tools. My problem is that I don't really see a good way to
handle that with llvm-provided command line abstractions (perhaps I'm
missing something there?)

What I basically want would be:
<tool> -d <build directory> <file1 ... fileN>
<tool> --use-args <clang command line>

But I don't want to roll my own command line parsing if possible.

> 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?

Cheers,
/Manuel

>
> -- 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
>