PATCH: Expose the 'file' that is associated with a compile database command

[Argyrios Kyrtzidis via cfe-commits]

In r247468, thanks for reviewing!

> On Sep 11, 2015, at 10:24 AM, Manuel Klimek via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> 
> Ok, looked at the original patch again, and if we're fixing the FixedCompilationDatabase to only insert the file when it actually produces a CompileCommand it seems to be fine.
> 
> On Fri, Sep 11, 2015 at 6:32 PM Argyrios Kyrtzidis <kyrtzidis at apple.com <mailto:kyrtzidis at apple.com>> wrote:
>> On Sep 11, 2015, at 12:21 AM, Manuel Klimek <klimek at google.com <mailto:klimek at google.com>> wrote:
>> 
>> On Thu, Sep 10, 2015 at 8:36 PM Argyrios Kyrtzidis <kyrtzidis at apple.com <mailto:kyrtzidis at apple.com>> wrote:
>>> On Sep 10, 2015, at 1:48 AM, Manuel Klimek <klimek at google.com <mailto:klimek at google.com>> wrote:
>>> 
>>> @@ -179,11 +185,13 @@ public:
>>>    /// \param Directory The base directory used in the FixedCompilationDatabase.
>>>    static FixedCompilationDatabase *loadFromCommandLine(int &Argc,
>>>                                                         const char *const *Argv,
>>> -                                                       Twine Directory = ".");
>>> +                                                       Twine Directory = ".",
>>> +                                                       Twine File = Twine());
>>>  
>>> A fixed compilation database returns the same command lines for all files, thus having a file in the function seems strange.
>> 
>> Ah ok, thanks for clarifying.
>> 
>>> 
>>> What exactly is the use case? So far, the compilation database has been designed for 2 use cases:
>>> 1. for a file, get the compile commands; the user already knows the file, no need to get the file
>>> 2. get all compile commands; for that, we have the getAllFiles() method, so a user can get all known files (for compilation databases that support that), and then get the compile command line.
>> 
>> It’s #2, I want to get all compile commands. But it seems really strange to me that the ‘file’ starts as a property of the compile command in the json file but then it gets dropped and I need to do work to re-associate the files with the compile commands again.
>> 
>> The JSON file format is one possible implementation for the compilation-database interface. The FixedCompilationDatabase is another one, that doesn't have any information on files.
>>  
>> I need to get a list of all the files and then for each one do a lookup to get the associated commands. I then have to maintain this association myself, passing a command along with its file separately or the structure that keeps track of the association.
>> 
>> It seems simpler to me to include the file that was associated with the command (if the compilation database supports that) along with the command, is there a downside I’m missing ?
>> 
>> Well, to me, it's a design question - if it also makes sense to have a CompileCommand without a file associated with it, putting the file in there, while convenient, is a design smell.
> 
> It can be optional to communicate that it may not be there. Note that, IMO, having multiple files and compile commands for them is the overwhelmingly most common use of the compilation database.
> 
>> That said, I'm happy to be convinced that I'm wrong :) I guess I don't see yet that keeping track of the files outside is more than one line of extra code.
> 
> Not sure what one line this is, I have to declare a map and then populate it, no ? And to do it in c-index-test it would take way more that one line.
> But it is also beyond populating a map, this has an effect on APIs using a CompileCommand. For example:
> 
> 	void doSomethingWithCompileCommand(const CompileCommand &cmd); 
> 
> Ah it would be useful to know the file that this command was associated with:
> 
> 	void doSomethingWithCompileCommand(const CompileCommand &cmd, StringRef filename); 
> 
> What do I have now ? This is a function taking a command and a string for the filename separately.
> Is this flexibility useful ? Does it make sense to pass any filename there ? No there’s only one filename that the command was associated with so this ‘flexibility’ only increases the complexity of using the function. And this can propagate to other function’s callees.
> This seems like a design smell to me.
> 
>> 
>> Cheers,
>> /Manuel
>>  
>> 
>>> 
>>> Thoughts?
>>> /Manuel
>>> 
>>> On Wed, Sep 9, 2015 at 9:36 PM Argyrios Kyrtzidis <kyrtzidis at apple.com <mailto:kyrtzidis at apple.com>> wrote:
>>> Hi,
>>> 
>>> The attached patch exposes the ‘file’ entry in a compilation database command, via the CompileCommand structure.
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150911/f0855c06/attachment-0001.html>