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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
