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

[Manuel Klimek via cfe-commits]

Test would be awesome :) Thx!

On Fri, Sep 11, 2015 at 10:44 PM Argyrios Kyrtzidis <kyrtzidis at apple.com>
wrote:

> 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>
> wrote:
>
>> On Sep 11, 2015, at 12:21 AM, Manuel Klimek <klimek at google.com> wrote:
>>
>> On Thu, Sep 10, 2015 at 8:36 PM Argyrios Kyrtzidis <kyrtzidis at apple.com>
>> wrote:
>>
>>> On Sep 10, 2015, at 1:48 AM, Manuel Klimek <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>
>>> 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/20150912/2c1034c5/attachment-0001.html>