[clang] Do not dump compilation-database entries for -E.

[tmroeder]

Summary

DumpCompilationDatabase currently produces entries for -E, but these entries do not work for libTooling-based tools. This change adds to the check at the beginning of DumpCompilationDatabase to make it return without writing anything if -E is specified.

[joerg]

I don't understand the point here. Why would you want to include pre-processing-only commands in the compilation database?

[tmroeder]

In D54657#1302086, @joerg wrote:

I don't understand the point here. Why would you want to include pre-processing-only commands in the compilation database?

Sorry for not being clear enough.

I don't want to include entries with -E. However, in the Linux kernel build, some build commands with -E occur after the regular build step, and that means that without the append option, the output .json file for those objects gets overwritten with a useless entry.

I originally thought that the simplest fix was to add a flag that appended to a json file instead of overwriting it. However, thinking about it some more, it would be nice if DumpCompilationDatabase refused to write such entries, since then I wouldn't care about the append version of this flag. I could drop the rest of this code (and fix the tests) if I could instead add the following after the check for -### at the top of DumpCompilationDatabase.

if (C.getArgs().hasArg(options::OPT_E))
  return;

What do you think? I don't want to break anyone who depends on the current behavior of -MJ, but I think it's reasonable for DumpCompilationDatabase to output only entries it can use.

[joerg]

I'm sorry, but it still sounds to me like you want to address badly written build rules by making the driver more complicated. I don't see that is a reasonable goal forward.

[tmroeder]

Change CompilationDatabase to not output entries that use -E.

These entries cannot be used by the tooling, so they can be skipped. This removes the other code and adds tests to make sure that neither -E or -### entries write a file.

[tmroeder]

In D54657#1303320, @joerg wrote:

I'm sorry, but it still sounds to me like you want to address badly written build rules by making the driver more complicated. I don't see that is a reasonable goal forward.

My goal is to build a compilation database for the Linux kernel so that I can use clang's excellent libTooling-based tools on it. I would like very much to do this in the least intrusive way possible both for the kernel and clang.

I also really like the -MJ flag, since I find it natural to ask the compiler to output a compilation database entry rather than depending on external tools. So, thank you for adding it.

Based on your feedback, I'm looking more deeply into the rules of the Linux Kbuild system to see if I can modify anything there. Preventing the -MJ flag from being passed to the -E invocations would prevent spurious entries from overwriting the entries that I need.

I think there's still an argument to be made for the code at hand, however, even though I understand that adding the check for -E adds code, hence makes the driver more complicated. In particular, I would argue that DumpCompilationDatabase should not output entries from pre-processing-only runs, since it appears to me that the other tools can't use those entries as input.

I've updated the patch to add only this check and tests now. Does this seem reasonable to you?

[tmroeder]

I found a better way to do this in the Linux kernel build.

lib/Driver/ToolChains/Clang.cpp

if (!Args.hasArg(options::OPT_fvisibility_EQ,
     CmdArgs.push_back("-fvisibility");
     CmdArgs.push_back("hidden");
   }
 }
 
 void Clang::DumpCompilationDatabase(Compilation &C, StringRef Filename,
                                     StringRef Target, const InputInfo &Output,
                                     const InputInfo &Input, const ArgList &Args) const {
-  // If this is a dry run, do not create the compilation database file.
-  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH))
+  // If this is a dry run or a pre-processing-only run, then do not create the
+  // compilation database file.
+  if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH, options::OPT_E))
     return;
 
   using llvm::yaml::escape;
   const Driver &D = getToolChain().getDriver();
 
   if (!CompilationDatabase) {
     std::error_code EC;
     auto File = llvm::make_unique<llvm::raw_fd_ostream>(Filename, EC, llvm::sys::fs::F_Text);

test/Driver/compilation_database.c

 // RUN: mkdir -p %t && cd %t
 // RUN: %clang -MD -MP --sysroot=somewhere -c -x c %s -xc++ %s -Wall -MJ - -no-canonical-prefixes 2>&1 | FileCheck %s
 // RUN: not %clang -c -x c %s -MJ %s/non-existant -no-canonical-prefixes 2>&1 | FileCheck --check-prefix=ERROR %s
+// RUN: %clang -E -c -x c %s -MJ E.json
+// RUN: test ! -f E.json
+// RUN: %clang -### -c -x c %s -MJ hash_hash_hash.json
+// RUN: test ! -f hash_hash_hash.json
 
 // CHECK: { "directory": "{{.*}}",  "file": "[[SRC:[^"]+[/|\\]compilation_database.c]]", "output": "compilation_database.o", "arguments": ["{{[^"]*}}clang{{[^"]*}}", "-xc", "[[SRC]]", "--sysroot=somewhere", "-c", "-Wall",{{.*}} "--target={{[^"]+}}"]},
 // CHECK: { "directory": "{{.*}}",  "file": "[[SRC:[^"]+[/|\\]compilation_database.c]]", "output": "compilation_database.o", "arguments": ["{{[^"]*}}clang{{[^"]*}}", "-xc++", "[[SRC]]", "--sysroot=somewhere", "-c", "-Wall",{{.*}} "--target={{[^"]+}}"]},
 // ERROR: error: compilation database '{{.*}}/non-existant' could not be opened:
 
 int main(void) {
   return 0;
 }