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