[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

Fri Apr 23 04:25:41 PDT 2021

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