[PATCH] D45006: [Tooling] A CompilationDatabase wrapper that infers header commands.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 4 04:57:14 PDT 2018


sammccall added inline comments.


================
Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:308
+               llvm::sys::path::extension(Filename).substr(1));
+      if (OldLang && NewLang != OldLang) {
+        Base.CommandLine.push_back("-x");
----------------
ilya-biryukov wrote:
> It feels like this heuristics only works for headers and files without extension (i.e. probably also headers).
> E.g., if we have a .cpp file and .c file, then trying to infer args for .c file from .cpp file is probably the wrong thing to do. And using Fortran flags for C or C++ is certainly the wrong thing to do.
> 
> It seems transferring flags between different languages is never fine, except for C/C++ headers. WDYT?
Urgh, you're right, this is dubious. But I think your suggestion is too narrow:
 - transferring flags between *.m, *.mm, and *.h seems fine (mixed m and mm isn't that uncommon I think). -x should be set on the headers but not on the m or mm files.
 - transferring between *.c and *.cc doesn't seem always wrong. Many -W, -I, and -D flags are shared (aren't these the most important ones?). Clearly adding -x is bad in this case.
 - yeah, fortran... we should drop those, but I'd wait for a report. compile_commands is a clang "standard" afaik, so putting fortran there doesn't make sense unless the build system doesn't know about languages.
 - also if we hard-ban some candidates, we no longer have the guarantee that we can pick a best candidate, which adds complexity

So I'd suggest:
 - in addition to the "implied language changed" guard, only add -x to certain languages
 - maybe we should reward same-language somehow. This is tricky, because if there's a compile command for one header, it might be quite unusual. Also there'll be *lots* of matches. Not sure how to best do this.



Repository:
  rC Clang

https://reviews.llvm.org/D45006





More information about the cfe-commits mailing list