jansvoboda11 accepted this revision. jansvoboda11 added a comment. This revision is now accepted and ready to land.
In D85802#2709832 <https://reviews.llvm.org/D85802#2709832>, @leonardchan wrote: > I added `cc1` invocations to the test and an extra one to assert that the > flag is actually making it to codegen. My point wasn't that you were using `%clang` in your tests instead of `%clang_cc1`. My point was that you were only checking that the driver accepts correct arguments and rejects invalid arguments, but were **not** checking that the argument actually influences codegen. This LGTM now though. In D85802#2709832 <https://reviews.llvm.org/D85802#2709832>, @leonardchan wrote: > I'm still unsure though how `GenerateLangArgs` should work though. There are use-cases in the compiler (e.g. dependency scanning), where we need to generate command-line from an instance of `CompilerInvocation`. This means that each function parsing command-line arguments (`ParseLangArgs`) needs to have a counterpart that does the opposite (`GenerateLangArgs`). We ensure all -cc1 options can be generated this way by doing a round-trip in assert builds: `command-line => ParseLangArgs => CompilerInvocation => GenerateLangArgs => command-line => ParseLangArgs => CompilerInvocation`. If some option doesn't get generated in `GenerateLangArgs`, it won't make it to the second `CompilerInvocation` which is used by the rest of the compiler. If we have good tests that verify the option actually does what it's supposed to, they will fail and we can fix the omission. In D85802#2709832 <https://reviews.llvm.org/D85802#2709832>, @leonardchan wrote: > Prior to this, I was still able to assert through both a driver and cc1 > invocation that I get the desired behavior in a debug build with assertions > enabled. That's odd. Does your `CMakeCache.txt` file have `CLANG_ROUND_TRIP_CC1_ARGS:BOOL=OFF` by any chance? It's possible it didn't get switched to `ON` when D97462 <https://reviews.llvm.org/D97462> landed. With the way the tests are written now, I'd expect them to fail in assert build without the code generating `-fcxx-abi=`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85802/new/ https://reviews.llvm.org/D85802 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
