Re: r248949 - Don't inherit availability information when implementing a protocol requirement.

[Hans Wennborg via cfe-commits]

On Fri, Oct 9, 2015 at 9:10 PM, Douglas Gregor  wrote:
>
>
> Sent from my iPhone
>
>> On Oct 9, 2015, at 2:17 PM, Hans Wennborg  wrote:
>>
>>> On Fri, Oct 9, 2015 at 1:46 PM, Douglas Gregor  wrote:
>>>
 On Oct 7, 2015, at 11:55 AM, Hans Wennborg  wrote:

 Hi Doug,

 On Wed, Sep 30, 2015 at 2:27 PM, Douglas Gregor via cfe-commits
  wrote:
> Author: dgregor
> Date: Wed Sep 30 16:27:42 2015
> New Revision: 248949
>
> URL: http://llvm.org/viewvc/llvm-project?rev=248949=rev
> Log:
> Don't inherit availability information when implementing a protocol 
> requirement.
>
> When an Objective-C method implements a protocol requirement, do not
> inherit any availability information from the protocol
> requirement. Rather, check that the implementation is not less
> available than the protocol requirement, as we do when overriding a
> method that has availability. Fixes rdar://problem/22734745.

 This is causing new warnings to fire in Chromium, and I'm not sure
 they're correct.

 For example:

 $ cat | build/bin/clang -c -x objective-c++ -
 #import 
 @protocol Foo
 @end
 @interface Bar : NSTextView {
 }
 @end
 @implementation Bar
 - (void)setMarkedText:(id)aString selectedRange:(NSRange)selRange {
   [super setMarkedText:aString selectedRange:selRange];
 }
 @end
 :9:10: warning: 'setMarkedText:selectedRange:' is deprecated:
 first deprecated in OS X 10.6 [-Wdeprecated-declarations]
   [super setMarkedText:aString selectedRange:selRange];
  ^
 /System/Library/Frameworks/AppKit.framework/Headers/NSInputManager.h:21:1:
 note: 'setMarkedText:selectedRange:' has been explicitly marked
 deprecated here
 - (void) setMarkedText:(id)aString selectedRange:(NSRange)selRange
 NS_DEPRECATED_MAC(10_0, 10_6);
 ^

 I don't really know Objective-C, but from what I understand,
 NSTextView implements both NSTextInput and NSTextInputClient.
 setMarkedText is deprecated in the former, but not the latter. So
 warning here is probably wrong?
>>>
>>> The warning is correct. We no longer infer that -[Bar 
>>> setMarkedText:selectedRange:] is deprecated simply because it matches up 
>>> with a deprecated requirement in the NSTextView protocol. You can mark this 
>>> method with
>>>
>>>NS_DEPRECATED_MAC(10_0, 10_6)
>>>
>>> (i.e., copy the availability information from the protocol requirement) to 
>>> get the old behavior.
>>
>>
>> Again, apologies for my Objective-C ignorance here, but don't you mean
>> the other way around?
>
> No, I have it correct.
>
>> Before this patch we did not warn that setMarkedText was deprecated,
>> but now we do. It seems we've gone from not inferring that it's
>> deprecated to doing so?
>
> NSTextView's setMarkedText is deprecated. Whether we diagnose a call to a 
> deprecated method depends on the context: inside another deprecated method we 
> suppress the diagnostic.
>
> That's where the inference comes in. We used to (incorrectly) infer that 
> Bar's setMarkedText was deprecated. That's wrong, and it suppressed the 
> deprecated warning for the call to the NSTextView version.

Oh, I see what you're saying. That makes sense. Thanks for explaining!

 - Hans
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r248949 - Don't inherit availability information when implementing a protocol requirement.

[Douglas Gregor via cfe-commits]

> On Oct 7, 2015, at 11:55 AM, Hans Wennborg  wrote:
> 
> Hi Doug,
> 
> On Wed, Sep 30, 2015 at 2:27 PM, Douglas Gregor via cfe-commits
>  wrote:
>> Author: dgregor
>> Date: Wed Sep 30 16:27:42 2015
>> New Revision: 248949
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=248949=rev
>> Log:
>> Don't inherit availability information when implementing a protocol 
>> requirement.
>> 
>> When an Objective-C method implements a protocol requirement, do not
>> inherit any availability information from the protocol
>> requirement. Rather, check that the implementation is not less
>> available than the protocol requirement, as we do when overriding a
>> method that has availability. Fixes rdar://problem/22734745.
> 
> This is causing new warnings to fire in Chromium, and I'm not sure
> they're correct.
> 
> For example:
> 
>  $ cat | build/bin/clang -c -x objective-c++ -
>  #import 
>  @protocol Foo
>  @end
>  @interface Bar : NSTextView {
>  }
>  @end
>  @implementation Bar
>  - (void)setMarkedText:(id)aString selectedRange:(NSRange)selRange {
>[super setMarkedText:aString selectedRange:selRange];
>  }
>  @end
>  :9:10: warning: 'setMarkedText:selectedRange:' is deprecated:
> first deprecated in OS X 10.6 [-Wdeprecated-declarations]
>[super setMarkedText:aString selectedRange:selRange];
>   ^
>  /System/Library/Frameworks/AppKit.framework/Headers/NSInputManager.h:21:1:
> note: 'setMarkedText:selectedRange:' has been explicitly marked
> deprecated here
>  - (void) setMarkedText:(id)aString selectedRange:(NSRange)selRange
> NS_DEPRECATED_MAC(10_0, 10_6);
>  ^
> 
> I don't really know Objective-C, but from what I understand,
> NSTextView implements both NSTextInput and NSTextInputClient.
> setMarkedText is deprecated in the former, but not the latter. So
> warning here is probably wrong?

The warning is correct. We no longer infer that -[Bar 
setMarkedText:selectedRange:] is deprecated simply because it matches up with a 
deprecated requirement in the NSTextView protocol. You can mark this method with

NS_DEPRECATED_MAC(10_0, 10_6)

(i.e., copy the availability information from the protocol requirement) to get 
the old behavior.

- Doug

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r248949 - Don't inherit availability information when implementing a protocol requirement.

[Hans Wennborg via cfe-commits]

On Fri, Oct 9, 2015 at 1:46 PM, Douglas Gregor  wrote:
>
>> On Oct 7, 2015, at 11:55 AM, Hans Wennborg  wrote:
>>
>> Hi Doug,
>>
>> On Wed, Sep 30, 2015 at 2:27 PM, Douglas Gregor via cfe-commits
>>  wrote:
>>> Author: dgregor
>>> Date: Wed Sep 30 16:27:42 2015
>>> New Revision: 248949
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=248949=rev
>>> Log:
>>> Don't inherit availability information when implementing a protocol 
>>> requirement.
>>>
>>> When an Objective-C method implements a protocol requirement, do not
>>> inherit any availability information from the protocol
>>> requirement. Rather, check that the implementation is not less
>>> available than the protocol requirement, as we do when overriding a
>>> method that has availability. Fixes rdar://problem/22734745.
>>
>> This is causing new warnings to fire in Chromium, and I'm not sure
>> they're correct.
>>
>> For example:
>>
>>  $ cat | build/bin/clang -c -x objective-c++ -
>>  #import 
>>  @protocol Foo
>>  @end
>>  @interface Bar : NSTextView {
>>  }
>>  @end
>>  @implementation Bar
>>  - (void)setMarkedText:(id)aString selectedRange:(NSRange)selRange {
>>[super setMarkedText:aString selectedRange:selRange];
>>  }
>>  @end
>>  :9:10: warning: 'setMarkedText:selectedRange:' is deprecated:
>> first deprecated in OS X 10.6 [-Wdeprecated-declarations]
>>[super setMarkedText:aString selectedRange:selRange];
>>   ^
>>  /System/Library/Frameworks/AppKit.framework/Headers/NSInputManager.h:21:1:
>> note: 'setMarkedText:selectedRange:' has been explicitly marked
>> deprecated here
>>  - (void) setMarkedText:(id)aString selectedRange:(NSRange)selRange
>> NS_DEPRECATED_MAC(10_0, 10_6);
>>  ^
>>
>> I don't really know Objective-C, but from what I understand,
>> NSTextView implements both NSTextInput and NSTextInputClient.
>> setMarkedText is deprecated in the former, but not the latter. So
>> warning here is probably wrong?
>
> The warning is correct. We no longer infer that -[Bar 
> setMarkedText:selectedRange:] is deprecated simply because it matches up with 
> a deprecated requirement in the NSTextView protocol. You can mark this method 
> with
>
> NS_DEPRECATED_MAC(10_0, 10_6)
>
> (i.e., copy the availability information from the protocol requirement) to 
> get the old behavior.


Again, apologies for my Objective-C ignorance here, but don't you mean
the other way around?

Before this patch we did not warn that setMarkedText was deprecated,
but now we do. It seems we've gone from not inferring that it's
deprecated to doing so?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r248949 - Don't inherit availability information when implementing a protocol requirement.

[Douglas Gregor via cfe-commits]

Sent from my iPhone

> On Oct 9, 2015, at 2:17 PM, Hans Wennborg  wrote:
> 
>> On Fri, Oct 9, 2015 at 1:46 PM, Douglas Gregor  wrote:
>> 
>>> On Oct 7, 2015, at 11:55 AM, Hans Wennborg  wrote:
>>> 
>>> Hi Doug,
>>> 
>>> On Wed, Sep 30, 2015 at 2:27 PM, Douglas Gregor via cfe-commits
>>>  wrote:
 Author: dgregor
 Date: Wed Sep 30 16:27:42 2015
 New Revision: 248949
 
 URL: http://llvm.org/viewvc/llvm-project?rev=248949=rev
 Log:
 Don't inherit availability information when implementing a protocol 
 requirement.
 
 When an Objective-C method implements a protocol requirement, do not
 inherit any availability information from the protocol
 requirement. Rather, check that the implementation is not less
 available than the protocol requirement, as we do when overriding a
 method that has availability. Fixes rdar://problem/22734745.
>>> 
>>> This is causing new warnings to fire in Chromium, and I'm not sure
>>> they're correct.
>>> 
>>> For example:
>>> 
>>> $ cat | build/bin/clang -c -x objective-c++ -
>>> #import 
>>> @protocol Foo
>>> @end
>>> @interface Bar : NSTextView {
>>> }
>>> @end
>>> @implementation Bar
>>> - (void)setMarkedText:(id)aString selectedRange:(NSRange)selRange {
>>>   [super setMarkedText:aString selectedRange:selRange];
>>> }
>>> @end
>>> :9:10: warning: 'setMarkedText:selectedRange:' is deprecated:
>>> first deprecated in OS X 10.6 [-Wdeprecated-declarations]
>>>   [super setMarkedText:aString selectedRange:selRange];
>>>  ^
>>> /System/Library/Frameworks/AppKit.framework/Headers/NSInputManager.h:21:1:
>>> note: 'setMarkedText:selectedRange:' has been explicitly marked
>>> deprecated here
>>> - (void) setMarkedText:(id)aString selectedRange:(NSRange)selRange
>>> NS_DEPRECATED_MAC(10_0, 10_6);
>>> ^
>>> 
>>> I don't really know Objective-C, but from what I understand,
>>> NSTextView implements both NSTextInput and NSTextInputClient.
>>> setMarkedText is deprecated in the former, but not the latter. So
>>> warning here is probably wrong?
>> 
>> The warning is correct. We no longer infer that -[Bar 
>> setMarkedText:selectedRange:] is deprecated simply because it matches up 
>> with a deprecated requirement in the NSTextView protocol. You can mark this 
>> method with
>> 
>>NS_DEPRECATED_MAC(10_0, 10_6)
>> 
>> (i.e., copy the availability information from the protocol requirement) to 
>> get the old behavior.
> 
> 
> Again, apologies for my Objective-C ignorance here, but don't you mean
> the other way around?

No, I have it correct. 

> Before this patch we did not warn that setMarkedText was deprecated,
> but now we do. It seems we've gone from not inferring that it's
> deprecated to doing so?

NSTextView's setMarkedText is deprecated. Whether we diagnose a call to a 
deprecated method depends on the context: inside another deprecated method we 
suppress the diagnostic. 

That's where the inference comes in. We used to (incorrectly) infer that Bar's 
setMarkedText was deprecated. That's wrong, and it suppressed the deprecated 
warning for the call to the NSTextView version. 

  - Doug

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r248949 - Don't inherit availability information when implementing a protocol requirement.

[Hans Wennborg via cfe-commits]

Hi Doug,

On Wed, Sep 30, 2015 at 2:27 PM, Douglas Gregor via cfe-commits
 wrote:
> Author: dgregor
> Date: Wed Sep 30 16:27:42 2015
> New Revision: 248949
>
> URL: http://llvm.org/viewvc/llvm-project?rev=248949=rev
> Log:
> Don't inherit availability information when implementing a protocol 
> requirement.
>
> When an Objective-C method implements a protocol requirement, do not
> inherit any availability information from the protocol
> requirement. Rather, check that the implementation is not less
> available than the protocol requirement, as we do when overriding a
> method that has availability. Fixes rdar://problem/22734745.

This is causing new warnings to fire in Chromium, and I'm not sure
they're correct.

For example:

  $ cat | build/bin/clang -c -x objective-c++ -
  #import 
  @protocol Foo
  @end
  @interface Bar : NSTextView {
  }
  @end
  @implementation Bar
  - (void)setMarkedText:(id)aString selectedRange:(NSRange)selRange {
[super setMarkedText:aString selectedRange:selRange];
  }
  @end
  :9:10: warning: 'setMarkedText:selectedRange:' is deprecated:
first deprecated in OS X 10.6 [-Wdeprecated-declarations]
[super setMarkedText:aString selectedRange:selRange];
   ^
  /System/Library/Frameworks/AppKit.framework/Headers/NSInputManager.h:21:1:
note: 'setMarkedText:selectedRange:' has been explicitly marked
deprecated here
  - (void) setMarkedText:(id)aString selectedRange:(NSRange)selRange
NS_DEPRECATED_MAC(10_0, 10_6);
  ^

I don't really know Objective-C, but from what I understand,
NSTextView implements both NSTextInput and NSTextInputClient.
setMarkedText is deprecated in the former, but not the latter. So
warning here is probably wrong?

Cheers,
Hans
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r248949 - Don't inherit availability information when implementing a protocol requirement.

[Douglas Gregor via cfe-commits]

Author: dgregor
Date: Wed Sep 30 16:27:42 2015
New Revision: 248949

URL: http://llvm.org/viewvc/llvm-project?rev=248949=rev
Log:
Don't inherit availability information when implementing a protocol requirement.

When an Objective-C method implements a protocol requirement, do not
inherit any availability information from the protocol
requirement. Rather, check that the implementation is not less
available than the protocol requirement, as we do when overriding a
method that has availability. Fixes rdar://problem/22734745.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/ARCMT/checking.m
cfe/trunk/test/SemaObjC/attr-availability.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=248949=248948=248949=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 30 16:27:42 
2015
@@ -2410,15 +2410,19 @@ def warn_availability_version_ordering :
 def warn_mismatched_availability: Warning<
   "availability does not match previous declaration">, InGroup;
 def warn_mismatched_availability_override : Warning<
-  "overriding method %select{introduced after|"
-  "deprecated before|obsoleted before}0 overridden method on %1 (%2 vs. %3)">, 
-  InGroup;
+  "%select{|overriding }4method %select{introduced after|"
+  "deprecated before|obsoleted before}0 "
+  "%select{the protocol method it implements|overridden method}4 "
+  "on %1 (%2 vs. %3)">, InGroup;
 def warn_mismatched_availability_override_unavail : Warning<
-  "overriding method cannot be unavailable on %0 when its overridden method is 
"
+  "%select{|overriding }1method cannot be unavailable on %0 when "
+  "%select{the protocol method it implements|its overridden method}1 is "
   "available">,
   InGroup;
 def note_overridden_method : Note<
   "overridden method is here">;
+def note_protocol_method : Note<
+  "protocol method is here">;
 
 // Thread Safety Attributes
 def warn_invalid_capability_name : Warning<

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=248949=248948=248949=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Sep 30 16:27:42 2015
@@ -2062,6 +2062,22 @@ public:
 TypeSourceInfo *TInfo);
   bool isIncompatibleTypedef(TypeDecl *Old, TypedefNameDecl *New);
 
+  /// \brief Describes the kind of merge to perform for availability
+  /// attributes (including "deprecated", "unavailable", and "availability").
+  enum AvailabilityMergeKind {
+/// \brief Don't merge availability attributes at all.
+AMK_None,
+/// \brief Merge availability attributes for a redeclaration, which 
requires
+/// an exact match.
+AMK_Redeclaration,
+/// \brief Merge availability attributes for an override, which requires
+/// an exact match or a weakening of constraints.
+AMK_Override,
+/// \brief Merge availability attributes for an implementation of
+/// a protocol requirement.
+AMK_ProtocolImplementation,
+  };
+
   /// Attribute merging methods. Return true if a new attribute was added.
   AvailabilityAttr *mergeAvailabilityAttr(NamedDecl *D, SourceRange Range,
   IdentifierInfo *Platform,
@@ -2070,7 +2086,7 @@ public:
   VersionTuple Obsoleted,
   bool IsUnavailable,
   StringRef Message,
-  bool Override,
+  AvailabilityMergeKind AMK,
   unsigned AttrSpellingListIndex);
   TypeVisibilityAttr *mergeTypeVisibilityAttr(Decl *D, SourceRange Range,
TypeVisibilityAttr::VisibilityType Vis,
@@ -2099,19 +2115,6 @@ public:
   OptimizeNoneAttr *mergeOptimizeNoneAttr(Decl *D, SourceRange Range,
   unsigned AttrSpellingListIndex);
 
-  /// \brief Describes the kind of merge to perform for availability
-  /// attributes (including "deprecated", "unavailable", and "availability").
-  enum AvailabilityMergeKind {
-/// \brief Don't merge availability attributes at all.
-AMK_None,
-/// \brief Merge availability attributes for a redeclaration, which 
requires
-/// an exact match.
-AMK_Redeclaration,
-/// \brief Merge availability attributes for an override, which requires
-/// an exact match or a weakening