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
