[clang-cl] Add -emit-ast to clang-cl driver

[thieta]

Summary

Also make the output of -emit-ast end up where /o points.
The same with .plist files from the static analyzer.

These are changes needed to make it possible to do CTU static
analysing work with clang-cl.

[thieta]

clang/include/clang/Driver/Options.td

834

Whops. My editor strikes again and removes a trailing space. Let me know if I should edit this out of this commit.

clang/lib/Driver/Driver.cpp

5629

I do wonder if this limitation here is really wanted. It makes clang-cl behave a bit different from clang/clang++. I can also split this part out in it's own commit if we feel like it's necessary.

[hans]

I'm unfamiliar with -emit-ast. Can you add some background on what this is for? What's CTU?

In any case this needs a test.

clang/include/clang/Driver/Options.td

834

Yes please. (Or land it separately.)

clang/lib/Driver/Driver.cpp

5629

What limitation are you referring to?

[thieta]

In D128409#3604460, @hans wrote:

I'm unfamiliar with -emit-ast. Can you add some background on what this is for? What's CTU?

CTU is cross translation unit. In this case the clang-static-analyzer can do analysis over several files - see the official docs that recommend that you build the .ast files with -emit-ast:
https://clang.llvm.org/docs/analyzer/user-docs/CrossTranslationUnit.html#manual-ctu-analysis

In any case this needs a test.

I can definitely add tests. What do you think needs a test here? that -emit-ast works the same with clang and clang-cl? Do we generally test the same arguments for all drivers even if they are expected to do the same thing?

clang/lib/Driver/Driver.cpp

5629

For the clang-cl options /Fo and /o we limit the exactly what should be written out to that file with TY_Object, TY_LTO_BC etc. But for the -o option we just dump everything with a few exceptions: https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/Driver.cpp#L5534

I haven't analyzed this method that carefully - but it seems like for most users using /o should be more or less analogous to -o?

[thieta]

Removed unintentional whitespace removal and landed that as a NFC

[hans]

In D128409#3604570, @thieta wrote:

In D128409#3604460, @hans wrote:

I'm unfamiliar with -emit-ast. Can you add some background on what this is for? What's CTU?

CTU is cross translation unit. In this case the clang-static-analyzer can do analysis over several files - see the official docs that recommend that you build the .ast files with -emit-ast:
https://clang.llvm.org/docs/analyzer/user-docs/CrossTranslationUnit.html#manual-ctu-analysis

Thanks! I wasn't aware of this.

In any case this needs a test.

I can definitely add tests. What do you think needs a test here? that -emit-ast works the same with clang and clang-cl? Do we generally test the same arguments for all drivers even if they are expected to do the same thing?

I think we should just check that clang-cl accepts the flag and passes the right thing to the -cc1 invocation (including that we get the output file name right).

clang/lib/Driver/Driver.cpp

5629

For /Fe, /Fo we need to match MSVC's behavior, so I think we do need these different cases. Note that we also allow /o for TY_Image in the if-else further down.

Perhaps TY_AST and Plist should have its own case?

[thieta]

Added tests and moved the check for TY_Plist and TY_AST to it's own if statement block

[hans]

It feels kind of weird to use the /Fo option for this that's for object files, but if that makes using the static analyzer more convenient that's okay I suppose.

clang/test/Driver/ast.c

30

This needs a -- before %s otherwise clang-cl might interpret the filename as a flag, e.g. if it starts with /Users which is common on Mac.

[thieta]

Fixed missing -- in test file

[thieta]

Fixed the test issue. Regarding /Fo bit - if you want I can change it to only handle /o in this if statement. I don't mind either way.

clang/test/Driver/ast.c

30

Fixed - will also fix the other tests in the same file as a NFC.

[hans]

Regarding /Fo bit - if you want I can change it to only handle /o in this if statement. I don't mind either way.

Just doing /o seems better. Thanks.

clang/test/Driver/ast.c

30

It's only an issue for clang-cl since it supports flags starting with slashes. I don't think there are other clang-cl tests in the file.

[thieta]

Just handle /o for AST and plist files

[thieta]

Updated commit message

[hans]

lgtm

clang/include/clang/Driver/Options.td

def z : Separate<["-"], "z">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass -z <arg> to the linker">, MetaVarName<"<arg>">,
   Group<Link_Group>;
 def offload_link : Flag<["--"], "offload-link">, Group<Link_Group>,
   HelpText<"Use the new offloading linker to perform the link job.">;
 def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass <arg> to the linker">, MetaVarName<"<arg>">,
   Group<Link_Group>;
 def Xoffload_linker : JoinedAndSeparate<["-"], "Xoffload-linker">,
   HelpText<"Pass <arg> to the offload linkers or the ones idenfied by -<triple>">,
   MetaVarName<"<triple> <arg>">, Group<Link_Group>;
 def Xpreprocessor : Separate<["-"], "Xpreprocessor">, Group<Preprocessor_Group>,
   HelpText<"Pass <arg> to the preprocessor">, MetaVarName<"<arg>">;
 def X_Flag : Flag<["-"], "X">, Group<Link_Group>;
 def X_Joined : Joined<["-"], "X">, IgnoredGCCCompat;
 def Z_Flag : Flag<["-"], "Z">, Group<Link_Group>;
 // FIXME: All we do with this is reject it. Remove.
 def Z_Joined : Joined<["-"], "Z">;

 def dumpversion : Flag<["-"], "dumpversion">;
 def dylib__file : Separate<["-"], "dylib_file">;
 def dylinker__install__name : JoinedOrSeparate<["-"], "dylinker_install_name">;
 def dylinker : Flag<["-"], "dylinker">;
 def dynamiclib : Flag<["-"], "dynamiclib">;
 def dynamic : Flag<["-"], "dynamic">, Flags<[NoArgumentUnused]>;
 def d_Flag : Flag<["-"], "d">, Group<d_Group>;
 def d_Joined : Joined<["-"], "d">, Group<d_Group>;
-def emit_ast : Flag<["-"], "emit-ast">,
+def emit_ast : Flag<["-"], "emit-ast">, Flags<[CoreOption]>,
   HelpText<"Emit Clang AST files for source inputs">;
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option, FC1Option, FlangOption]>, Group<Action_Group>,
   HelpText<"Use the LLVM representation for assembler and object files">;
 def emit_interface_stubs : Flag<["-"], "emit-interface-stubs">, Flags<[CC1Option]>, Group<Action_Group>,
   HelpText<"Generate Interface Stub Files.">;
 def emit_merged_ifs : Flag<["-"], "emit-merged-ifs">,
   Flags<[CC1Option]>, Group<Action_Group>,
   HelpText<"Generate Interface Stub Files, emit merged text not binary.">;

clang/lib/Driver/Driver.cpp

if (isa<DsymutilJobAction>(JA) && C.getArgs().hasArg(options::OPT_dsym_dir)) {
     BaseName = BasePath;
   else
     BaseName = llvm::sys::path::filename(BasePath);
 
   // Determine what the derived output name should be.
   const char *NamedOutput;
 
   if ((JA.getType() == types::TY_Object || JA.getType() == types::TY_LTO_BC) &&
       C.getArgs().hasArg(options::OPT__SLASH_Fo, options::OPT__SLASH_o)) {
     // The /Fo or /o flag decides the object filename.
     StringRef Val =
         C.getArgs()
             .getLastArg(options::OPT__SLASH_Fo, options::OPT__SLASH_o)
             ->getValue();
     NamedOutput =
         MakeCLOutputFilename(C.getArgs(), Val, BaseName, types::TY_Object);
   } else if (JA.getType() == types::TY_Image &&
if (IsCLMode()) {
         Output.append(BoundArch);
       }
       if (UseOutExtension)
         Output += ".out";
       NamedOutput = C.getArgs().MakeArgString(Output.c_str());
     }
   } else if (JA.getType() == types::TY_PCH && IsCLMode()) {
     NamedOutput = C.getArgs().MakeArgString(GetClPchPath(C, BaseName));
+  } else if ((JA.getType() == types::TY_Plist || JA.getType() == types::TY_AST) &&
+             C.getArgs().hasArg(options::OPT__SLASH_Fo, options::OPT__SLASH_o)) {
+    StringRef Val =
+        C.getArgs()
+            .getLastArg(options::OPT__SLASH_Fo, options::OPT__SLASH_o)
+            ->getValue();
+    NamedOutput =
+        MakeCLOutputFilename(C.getArgs(), Val, BaseName, types::TY_Object);
   } else {
     const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
     assert(Suffix && "All types used for output should have a suffix.");
 
     std::string::size_type End = std::string::npos;
     if (!types::appendSuffixForType(JA.getType()))
       End = BaseName.rfind('.');
     SmallString<128> Suffixed(BaseName.substr(0, End));

clang/test/Driver/ast.c

 // COMPILE-AST-PHASES: 2: backend, {1}, assembler
 // COMPILE-AST-PHASES: 3: assembler, {2}, object
 // COMPILE-AST-PHASES-NOT: 4:
 // COMPILE-AST-PHASES: END
 
 // FIXME: There is a problem with compiling AST's in that the input language is
 // not available for use by other tools (for example, to automatically add
 // -lstdc++). We may need -x [objective-]c++-ast and all that goodness. :(
+
+// Also check clang-cl since the driver is slightly different
+// RUN: %clang_cl -ccc-print-phases -emit-ast %s 2> %t
+// RUN: echo 'END' >> %t
+// RUN: FileCheck -check-prefix EMIT-AST-PHASES-CLANGCL -input-file %t %s
+
+// EMIT-AST-PHASES-CLANGCL: 0: input,
+// EMIT-AST-PHASES-CLANGCL: , c
+// EMIT-AST-PHASES-CLANGCL: 1: preprocessor, {0}, cpp-output
+// EMIT-AST-PHASES-CLANGCL: 2: compiler, {1}, ast
+// EMIT-AST-PHASES-CLANGCL-NOT: 3:
+// EMIT-AST-PHASES-CLANGCL: END

clang/test/Driver/cl-outputs.c

 
 // RUN: %clang_cl /Fomydir/ -### -- %s %s 2>&1 | FileCheck -check-prefix=CHECK-MULTIPLESOURCEOK %s
 // CHECK-MULTIPLESOURCEOK: "-o" "mydir{{[/\\]+}}cl-outputs.obj"
 
 // RUN: %clang_cl /Fo -### -- %s %s 2>&1 | FileCheck -check-prefix=CHECK-MULTIPLESOURCEOK2 %s
 // CHECK-MULTIPLESOURCEOK2: "-o" "cl-outputs.obj"
 // CHECK-MULTIPLESOURCEOK2: "-o" "cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /Fotest.ast -###  -- %s  2>&1 | FileCheck -check-prefix=FoASTNAME %s
+// FoASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /Fotest.plist -### -- %s 2>&1 | FileCheck -check-prefix=FoPLIST %s
+// FoPLIST:  "-o" "test.plist"
+
 // RUN: %clang_cl /c /oa -### -- %s 2>&1 | FileCheck -check-prefix=oNAME1 %s
 // oNAME1:  "-o" "a.obj"
 
 // RUN: %clang_cl /c /o a -### -- %s 2>&1 | FileCheck -check-prefix=oNAME2 %s
 // oNAME2:  "-o" "a.obj"
 
 // RUN: %clang_cl /c /oa.ext /ob.ext -### -- %s 2>&1 | FileCheck -check-prefix=oNAMEEXT1 %s
 // oNAMEEXT1:  "-o" "b.ext"

 // oMISSINGARG: error: argument to '/o' is missing (expected 1 value)
 
 // RUN: %clang_cl /c /omydir/ -### -- %s %s 2>&1 | FileCheck -check-prefix=CHECK-oMULTIPLESOURCEOK1 %s
 // CHECK-oMULTIPLESOURCEOK1: "-o" "mydir{{[/\\]+}}cl-outputs.obj"
 
 // RUN: %clang_cl /c /o mydir/ -### -- %s %s 2>&1 | FileCheck -check-prefix=CHECK-oMULTIPLESOURCEOK2 %s
 // CHECK-oMULTIPLESOURCEOK2: "-o" "mydir{{[/\\]+}}cl-outputs.obj"
 
+// RUN: %clang_cl -emit-ast /otest.ast -###  -- %s  2>&1 | FileCheck -check-prefix=oASTNAME %s
+// oASTNAME:  "-o" "test.ast"
+
+// RUN: %clang_cl --analyze /otest.plist -###  -- %s  2>&1 | FileCheck -check-prefix=oPLIST %s
+// oPLIST:  "-o" "test.plist"
 
 // RUN: %clang_cl /c /obar /Fofoo -### -- %s 2>&1 | FileCheck -check-prefix=FooRACE1 %s
 // FooRACE1: "-o" "foo.obj"
 
 // RUN: %clang_cl /c /Fofoo /obar -### -- %s 2>&1 | FileCheck -check-prefix=FooRACE2 %s
 // FooRACE2: "-o" "bar.obj"