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 > <mailto: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 <mailto: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 >> <mailto:kyrtzi...@apple.com>> wrote: >>> On Sep 11, 2015, at 12:21 AM, Manuel Klimek <kli...@google.com >>> <mailto:kli...@google.com>> wrote: >>> >>> On Thu, Sep 10, 2015 at 8:36 PM Argyrios Kyrtzidis <kyrtzi...@apple.com >>> <mailto:kyrtzi...@apple.com>> wrote: >>>> On Sep 10, 2015, at 1:48 AM, Manuel Klimek <kli...@google.com >>>> <mailto: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 >>>> <mailto: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 <mailto:cfe-commits@lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> <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
