r199682 - ObjectiveC driver. reinstate -fno-objc-legacy-dispatch behavior

[Bob Wilson]

On Jan 21, 2014, at 4:13 PM, jahanian <fjahanian at apple.com> wrote:

> 
> On Jan 21, 2014, at 4:08 PM, Bob Wilson <bob.wilson at apple.com> wrote:
> 
>> 
>> On Jan 20, 2014, at 11:32 AM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>> 
>>> Author: fjahanian
>>> Date: Mon Jan 20 13:32:33 2014
>>> New Revision: 199682
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=199682&view=rev
>>> Log:
>>> ObjectiveC driver. reinstate -fno-objc-legacy-dispatch behavior 
>>> when the deployment target is 10.5. // rdar://15852259
>>> 
>>> Modified:
>>>  cfe/trunk/include/clang/Basic/ObjCRuntime.h
>>>  cfe/trunk/lib/Driver/Tools.cpp
>>>  cfe/trunk/test/Driver/darwin-objc-defaults.m
>>> 
>>> Modified: cfe/trunk/include/clang/Basic/ObjCRuntime.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/ObjCRuntime.h?rev=199682&r1=199681&r2=199682&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Basic/ObjCRuntime.h (original)
>>> +++ cfe/trunk/include/clang/Basic/ObjCRuntime.h Mon Jan 20 13:32:33 2014
>>> @@ -99,6 +99,15 @@ public:
>>>         Arch == llvm::Triple::x86_64)
>>>       return false;
>>>   }
>>> +    else if (isNeXTFamily() && getVersion().getMajor() == 10) {
>>> +      unsigned minor = 0;
>>> +      if (getVersion().getMinor().hasValue())
>>> +        minor = getVersion().getMinor().getValue();
>>> +      if (minor == 5 &&
>>> +          (Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64))
>>> +        return false;
>>> +    }
>>> +    // Except for deployment target of 10.5,
>>>   // Mac runtimes use legacy dispatch everywhere now.
>>>   return true;
>>> }
>> 
>> isNextFamily() returns true for iOS. We haven’t gotten to iOS 10 yet, but in the interest of future-proofing, I think this needs to check specifically for OS X.
> OK.
> 
>> 
>> The version check can just be "getVersion() == VersionTuple(10, 5)", can’t it?
> 
> No. This fails for, say, 10.5.2 (but not for pure 10.5).

OK, well, how about checking for 10.5 <= version < 10.6?  I just find the getVersion().getMinor().hasValue() code really hard to read.

> 
>> 
>> I’m not sure about the architecture check.  Mac OS X 10.5 didn’t support anything except x86 and x86_64. Prior to svn 192719, the check here was:
>> 
>> if (isNeXTFamily() && isNonFragile())
>>      return Arch != llvm::Triple::x86_64;
>> 
>> With this patch, you’re not checking isNonFragile() and you’re treating x86 and x86_64 the same.  That doesn’t seem right.
> 
> You are right, I will restore the exact old patch for this.
> - Fariborz
> 
>> 
>>> 
>>> Modified: cfe/trunk/lib/Driver/Tools.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=199682&r1=199681&r2=199682&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/Driver/Tools.cpp (original)
>>> +++ cfe/trunk/lib/Driver/Tools.cpp Mon Jan 20 13:32:33 2014
>>> @@ -3324,9 +3324,10 @@ void Clang::ConstructJob(Compilation &C,
>>> ObjCRuntime objcRuntime = AddObjCRuntimeArgs(Args, CmdArgs, rewriteKind);
>>> 
>>> // -fobjc-dispatch-method is only relevant with the nonfragile-abi, and
>>> -  // legacy is the default. Next runtime is always legacy dispatch and
>>> -  // -fno-objc-legacy-dispatch gets ignored silently.
>>> -  if (objcRuntime.isNonFragile() && !objcRuntime.isNeXTFamily()) {
>>> +  // legacy is the default. Except for deployment taget of 10.5,
>>> +  // next runtime is always legacy dispatch and -fno-objc-legacy-dispatch
>>> +  // gets ignored silently.
>>> +  if (objcRuntime.isNonFragile()) {
>>>   if (!Args.hasFlag(options::OPT_fobjc_legacy_dispatch,
>>>                     options::OPT_fno_objc_legacy_dispatch,
>>>                     objcRuntime.isLegacyDispatchDefaultForArch(
>>> 
>>> Modified: cfe/trunk/test/Driver/darwin-objc-defaults.m
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-objc-defaults.m?rev=199682&r1=199681&r2=199682&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/test/Driver/darwin-objc-defaults.m (original)
>>> +++ cfe/trunk/test/Driver/darwin-objc-defaults.m Mon Jan 20 13:32:33 2014
>>> @@ -37,6 +37,7 @@
>>> 
>>> // CHECK-CHECK-X86_64_OSX10_5: "-cc1"
>>> // CHECK-CHECK-X86_64_OSX10_5: -fobjc-runtime=macosx-10.5
>>> +// CHECK-CHECK-X86_64_OSX10_5: -fobjc-dispatch-method=non-legacy
>>> // CHECK-CHECK-X86_64_OSX10_5: darwin-objc-defaults
>>> 
>>> // RUN: %clang -target x86_64-apple-darwin10 -S -### %s \
>>> 
>>> 
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> 
>