r321099 - [driver][darwin] Take the OS version specified in "-target" as the target

Duncan Exon Smith via cfe-commits cfe-commits at lists.llvm.org
Sun Dec 24 18:23:10 PST 2017


> On Dec 21, 2017, at 16:39, Alex L <arphaman at gmail.com> wrote:
> 
> 
> 
>> On 21 December 2017 at 12:34, James Y Knight <jyknight at google.com> wrote:
>> I totally agree with moving towards eliminating the -m<os>-version-min flags, it's much better to put it in the target, and will clean up a lot of cruft in the driver, eventually.
>> 
>> Now -- we (or anyone else who runs into this) can simply start specifying the version in both locations ("-target x86_64-apple-ios10 -mios-simulator-version-min=10"), so as to work with both new and old clang, and be closer to the ultimate goal of having only -target. That's an overall nicer workaround to suggest than switching to -darwin. But, yea, there's no need for *temporary* patch to fix things just for us.
>> 
>> However, I do not understand why you're against committing the patch you mention as option #2 -- that seems like it'd be best for all users, by preserving compatibility with existing command-lines. So, I'd still like that change to be committed, permanently, not temporarily. I'm sure we can't be the only ones running clang like "-target x86_64-apple-ios -mios-simulator-version-min=10", and it seems unfortunate and unnecessary to break that, even if it can be worked around.
>> 
>> In the future, I'd hope the -m<os>-version-min arguments can be deprecated more and more -- warning whenever you use them to modify the platform or version at all, rather just on specification conflict; then, warn anytime you use them at all; then, remove them. But in the meantime, it seems strictly better to preserve compatibility, don't you think?
> 
> + Duncan
> 
> Thanks, I think your argument is convincing. I think I will commit the change that you're proposing in the near future if there are no further objections.

SGTM. 

> 
>  
>> 
>> 
>> 
>> On Dec 21, 2017 2:11 PM, "Alex L" <arphaman at gmail.com> wrote:
>> Thanks for raising your concerns.
>> 
>> We decided to avoid -m<os>-version-min flag in favor of -target to simplify the driver logic and to encourage the adoption of -target. Now after r321145 we only warn about -m<os>-version-min flag when the OS version specified in it is different to the OS version specified in target, or when target has no OS version.
>> 
>> There are two possible solutions here:
>> 1) You can still use -target with -mios-simulator-version-min as before but you'd have to use '-target=x86_64-apple-darwin' to ensure that the iOS version specified by  '-mios-simulator-version-min' is used.
>> 2) I also do have a patch that implements the logic that you propose (use the OS version in -m<os>-version-min flag if target has none). If you believe that the first solution is not suitable for your code then I can commit it. At the same time I believe that we would rather not use this patch, but if it's urgent for your projects then maybe I can land it now and then we can establish some sort of timeline for when it can be reverted?
>> 
>> Thanks,
>> Alex
>> 
>> 
>>> On 21 December 2017 at 08:00, James Y Knight <jyknight at google.com> wrote:
>>> I think if a version number isn't explicitly specified in the -target value, the value from -m<platform>-version-min ought to still be used, as it was before.
>>> 
>>> Currently, clang will ignore the -m<platform>-version-min version number if the target has a particular OS specified, even if it has no version number as part of it.
>>> 
>>> (We should be able to workaround this change backwards-compatibly by specifying in both the -target argument and in the -m<platform>-version-min arguments, but I do think the behavior should be fixed.)
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171224/a36a41b9/attachment.html>


More information about the cfe-commits mailing list