dexonsmith requested changes to this revision.
dexonsmith added inline comments.
This revision now requires changes to proceed.
================
Comment at: lib/Driver/ToolChains/Darwin.cpp:1234-1276
+ // The -target option specifies the deployment target when
+ // -m<os>-version-min is not given and the OS version is present in the
+ // target.
+ if (Args.hasArg(options::OPT_target) && getTriple().getOSMajorVersion() &&
+ getTriple().isOSDarwin()) {
+ unsigned Major, Minor, Micro;
+ auto RewriteVersionMinArg = [&](options::ID O) -> Arg * {
----------------
This logic seems to be duplicated from the `-target` handling below. Perhaps
this would be simpler if `-target` were processed first.
I suggest the following refactoring in an NFC prep patch:
- First, check for a `-target` option. If it exists, extract all the bits.
- Then, handle the logic for `-arch`, `-m*-version-min`, and `-isysroot`
(matching the current semantics, which I think are "override `-target`").
In a follow-up patch, change `-isysroot`/`SDKROOT` not to override a
`-target`-specified environment (the semantic change from this patch). After
the refactoring, I suspect this will be trivial.
We should also warn/error when `-arch` disagrees with `-target`, and likely the
same for `-m*-version-min`. I suspect these follow-ups will also be trivial.
================
Comment at: lib/Driver/ToolChains/Darwin.cpp:1273
+ default:
+ llvm_unreachable("invalid Os");
+ }
----------------
I'd spell this `"invalid OS"`.
Repository:
rC Clang
https://reviews.llvm.org/D40998
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
