[PATCH] D128409: [clang-cl] Add -emit-ast to clang-cl driver

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D128409#3604570 <https://reviews.llvm.org/D128409#3604570>, @thieta wrote:

> In D128409#3604460 <https://reviews.llvm.org/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).



================
Comment at: clang/lib/Driver/Driver.cpp:5629
+  if ((JA.getType() == types::TY_Object || JA.getType() == types::TY_LTO_BC ||
+       JA.getType() == types::TY_AST || JA.getType() == types::TY_Plist) &&
       C.getArgs().hasArg(options::OPT__SLASH_Fo, options::OPT__SLASH_o)) {
----------------
thieta wrote:
> hans wrote:
> > thieta wrote:
> > > 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.
> > What limitation are you referring to?
> 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`?
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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128409/new/

https://reviews.llvm.org/D128409