r329110 - [driver][darwin] Do not infer -simulator environment for non-simulator SDKs

[Chandler Carruth via cfe-commits]

On Tue, Apr 3, 2018 at 4:01 PM Alex L via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> On 3 April 2018 at 14:30, Chandler Carruth <chandlerc at gmail.com> wrote:
>
>> On Tue, Apr 3, 2018 at 1:52 PM Alex Lorenz via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: arphaman
>>> Date: Tue Apr  3 13:50:05 2018
>>> New Revision: 329110
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=329110&view=rev
>>> Log:
>>> [driver][darwin] Do not infer -simulator environment for non-simulator
>>> SDKs
>>>
>>
>> I know you added a REQUIRES line to these tests, but I think there is a
>> much better way:
>>
>>
>>> --- cfe/trunk/test/Driver/darwin-sdkroot.c (original)
>>> +++ cfe/trunk/test/Driver/darwin-sdkroot.c Tue Apr  3 13:50:05 2018
>>> @@ -51,12 +51,21 @@
>>>  // CHECK-IPHONE: "-triple" "arm64-apple-ios8.0.0"
>>>  // CHECK-IPHONE: ld
>>>  // CHECK-IPHONE: "-iphoneos_version_min" "8.0.0"
>>> +// RUN: env SDKROOT=%t/SDKs/iPhoneOS8.0.0.sdk %clang %s -### 2>&1 \
>>>
>>
>> Instead of just running %clang, actually pass the `-target` you want to
>> it like we do in the below invocation and the other invocations in this
>> file.
>>
>> We shouldn't lose driver testing on other systems as long as you can
>> specify the desired target.
>>
>
>
> Hi Chandler!
>
> Thanks for pointing this out! We actually can't use -target here because
> when -target is specified, Darwin's driver won't infer the triple's
> environment from the SDKROOT. So this test covers the path in the driver
> that won't be taken when -target is specified.
>

Ah, I see, that's where the inference is triggered. Sure.


>
> You've made a good point about losing testing though. I can split out this
> test into the original file (with -target use) and the new tests which
> can't use -target and are Darwin specific to ensure we won't loose the
> existing coverage. I will commit a follow-up commit that does this.
>

Sure, makes sense to factor out inference-based tests where possible and
cover as much as you can w/o inference.

Thanks for explaining!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180403/ac045119/attachment.html>