[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