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

[Manuel Klimek via cfe-commits]

Thanks!

On Wed, Sep 16, 2015 at 11:30 AM Argyrios Kyrtzidis <kyrtzidis at apple.com>
wrote:

> In r247832.
>
> On Sep 12, 2015, at 5:19 AM, Manuel Klimek <klimek at google.com> wrote:
>
> 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/20150916/f1c1fb0d/attachment.html>