[Driver] Allow use of -fsyntax-only together with -MJ

[dstenb]

Summary

When using -MJ together with -fsyntax-only, clang would hit an assert in
DumpCompilationDatabase() when trying to get the filename for the output
field. This patch fixes that by amending DumpCompilationDatabase() so
that it accepts the `nothing' output class, and it will in that case
simply omit the output field.

The JSON Compilation Database Format Specification specifies that the
output field is optional.

[joerg]

IMO we should explicitly error out. That combination is nonsense to me. Creating useless JSON database fragments is not an improvement.

[dstenb]

Downstream we use -MJ in a bit of an idiosyncratic way, as we're in a transition period where we, for a subset of the code base, only use the clang frontend for diagnostics, and not for the code generation. However, if you don't think that using -fsyntax-only and -MJ makes sense in any upstream application, I'll drop from this change. I'm leaving the assertion as-is.

[joerg]

Oh, we certainly should never be hitting an assertion on front-end flags. As such, there is a problem to fix here. I still maintain that the combination of flags is non-sense, so the question is:

(1) Should be silently ignore -MJ? That would be useful for your use case, but seems to violate POLA.

(2) Should be error out explicitly when no output is known? This is more harsh, but seems to provide more consistent behavior for -MJ.

[dstenb]

Our legacy frontend does not support -MJ, so when using that frontend for code generation, we also invoke clang with -MJ, and at the same use -fsyntax-only to get the improved diagnostics that clang provides. This is idiosyncratic and probably hacky, I know, but it works well enough to for example for getting access to defines and include flags from the compilation database, and being able to run clang-tidy. So (1) does not fit our use case, unfortunately.

Our way of running should of course in itself not affect what is done upstream, but in general I don't see why someone shouldn't be allowed to do, for example, the following in their build chain:

clang -MJ - -DFOO=usual-value foo.c -o foo.out # code generation
clang -MJ - -DFOO=unusual-value -fsyntax-value foo.c # quick sanity check

and then run a tool like clang-tidy with the resulting compilation database.

Disregarding the above, I agree about (1)'s POLA issue, so out of those two options the latter seems better to me.

EDIT: Fixed a typo and reworded some parts slightly.

lib/Driver/ToolChains/Clang.cpp

if (!CompilationDatabase) {
     CompilationDatabase = std::move(File);
   }
   auto &CDB = *CompilationDatabase;
   SmallString<128> Buf;
   if (llvm::sys::fs::current_path(Buf))
     Buf = ".";
   CDB << "{ \"directory\": \"" << escape(Buf) << "\"";
   CDB << ", \"file\": \"" << escape(Input.getFilename()) << "\"";
+  if (!Output.isNothing())
     CDB << ", \"output\": \"" << escape(Output.getFilename()) << "\"";
   CDB << ", \"arguments\": [\"" << escape(D.ClangExecutable) << "\"";
   Buf = "-x";
   Buf += types::getTypeName(Input.getType());
   CDB << ", \"" << escape(Buf) << "\"";
   if (!D.SysRoot.empty() && !Args.hasArg(options::OPT__sysroot_EQ)) {
     Buf = "--sysroot=";
     Buf += D.SysRoot;
     CDB << ", \"" << escape(Buf) << "\"";

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 %s --sysroot=somewhere -MJ - -fsyntax-only -no-canonical-prefixes 2>&1 | FileCheck --check-prefix=FSYNTAX-ONLY %s
 
 // 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:
+// FSYNTAX-ONLY: { "directory": "{{.*}}",  "file": "[[SRC:[^"]+[/|\\]compilation_database.c]]", "arguments": ["{{[^"]*}}clang{{[^"]*}}", "-xc", "[[SRC]]", "--sysroot=somewhere", {{.*}} "--target={{[^"]+}}"]},
 
 int main(void) {
   return 0;
 }