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

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.


================
Comment at: clang/include/clang/Basic/TargetCXXABI.h:39
 
+  static const auto &getABIMap() {
+    static llvm::StringMap<Kind> ABIMap = {
----------------
The option UI should use lowercase values by default, or else just do case-insensitive matching.



================
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:274
+
+  if (DriverArgs.getLastArgValue(options::OPT_fcxx_abi_EQ) == "Fuchsia")
+    CC1Args.push_back("-fexperimental-relative-c++-abi-vtables");
----------------
This should match "fuchsia", either only lowercase or case-insensitive.  IMHO it seems wise to handle this in some way such that individual tests like this are not free-form string comparisons that could have typos, but test the TargetCXXABI enum value after decoding from string.

This should be a separate change.  We'll need to do staged roll-out, first of a compiler where `-fc++abi=compat` (or itanium whatever it's called) is available (complete with multilibs et al), and then later of a compiler where `-fc++abi=fuchsia` (and the default for *-fuchsia targets) has changed meaning.



================
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:275
+  if (DriverArgs.getLastArgValue(options::OPT_fcxx_abi_EQ) == "Fuchsia")
+    CC1Args.push_back("-fexperimental-relative-c++-abi-vtables");
 }
----------------
It's surprising to me that this is the way to do this.  Isn't there code in the actual front end that tests the TargetCXXABI value?  That seems like the place where it makes sense to have Fuchsia imply specific settings, rather than in the driver.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802