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

[Manuel Klimek]

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.

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

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.

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
>