Thanks! On Wed, Sep 16, 2015 at 11:30 AM Argyrios Kyrtzidis <kyrtzi...@apple.com> wrote:
> In r247832. > > On Sep 12, 2015, at 5:19 AM, Manuel Klimek <kli...@google.com> wrote: > > Test would be awesome :) Thx! > > On Fri, Sep 11, 2015 at 10:44 PM Argyrios Kyrtzidis <kyrtzi...@apple.com> > wrote: > >> In r247468, thanks for reviewing! >> >> On Sep 11, 2015, at 10:24 AM, Manuel Klimek via cfe-commits < >> cfe-commits@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 <kyrtzi...@apple.com> >> wrote: >> >>> On Sep 11, 2015, at 12:21 AM, Manuel Klimek <kli...@google.com> wrote: >>> >>> On Thu, Sep 10, 2015 at 8:36 PM Argyrios Kyrtzidis <kyrtzi...@apple.com> >>> wrote: >>> >>>> On Sep 10, 2015, at 1:48 AM, Manuel Klimek <kli...@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 <kyrtzi...@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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
