[cfe-commits] r172567 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclAttr.cpp lib/Sema/SemaDeclObjC.cpp test/SemaObjC/attr-availability.m

Douglas Gregor dgregor at apple.com
Tue Jan 15 16:55:24 PST 2013


On Jan 15, 2013, at 3:25 PM, Jordan Rose <jordan_rose at apple.com> wrote:

> 
> On Jan 15, 2013, at 14:43 , Douglas Gregor <dgregor at apple.com> wrote:
> 
>> Author: dgregor
>> Date: Tue Jan 15 16:43:08 2013
>> New Revision: 172567
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=172567&view=rev
>> Log:
>> When checking availability attributes for consistency between an
>> overriding and overridden method, allow the overridden method to have
>> a narrower contract (introduced earlier, deprecated/obsoleted later)
>> than the overriding method. Fixes <rdar://problem/12992023>.
> 
> I think you mean "allow the overridding method…" I spent five minutes trying to figure out if the comment as written really made sense.

Yep, my apologies for the typo.

> Unfortunately, the test case seems backwards as well for warn_mismatched_availability_override_unavail, but correct for both variants of warn_mismatched_availability_override.

Good eye, thanks! r172586.

	- Doug

> 
>> 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/lib/Sema/SemaDeclObjC.cpp
>>    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=172567&r1=172566&r2=172567&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Jan 15 16:43:08 2013
>> @@ -1879,6 +1879,16 @@
>>   "attribute ignored">, InGroup<Availability>;
>> def warn_mismatched_availability: Warning<
>>   "availability does not match previous declaration">, InGroup<Availability>;
>> +def warn_mismatched_availability_override : Warning<
>> +  "overriding method %select{introduced after|"
>> +  "deprecated before|obsoleted before}0 overridden method on %1 (%2 vs. %3)">, 
>> +  InGroup<Availability>;
>> +def warn_mismatched_availability_override_unavail : Warning<
>> +  "overriding method cannot be unavailable on %0 when its overridden method is "
>> +  "available">,
>> +  InGroup<Availability>;
>> +def note_overridden_method : Note<
>> +  "overridden method is here">;
>> 
>> // Thread Safety Attributes
>> def warn_thread_attribute_ignored : Warning<
>> 
>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=172567&r1=172566&r2=172567&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>> +++ cfe/trunk/include/clang/Sema/Sema.h Tue Jan 15 16:43:08 2013
>> @@ -1677,7 +1677,8 @@
>>                                           VersionTuple Deprecated,
>>                                           VersionTuple Obsoleted,
>>                                           bool IsUnavailable,
>> -                                          StringRef Message);
>> +                                          StringRef Message,
>> +                                          bool Override);
>>   VisibilityAttr *mergeVisibilityAttr(Decl *D, SourceRange Range,
>>                                       VisibilityAttr::VisibilityType Vis);
>>   DLLImportAttr *mergeDLLImportAttr(Decl *D, SourceRange Range);
>> @@ -1685,10 +1686,24 @@
>>   FormatAttr *mergeFormatAttr(Decl *D, SourceRange Range, StringRef Format,
>>                               int FormatIdx, int FirstArg);
>>   SectionAttr *mergeSectionAttr(Decl *D, SourceRange Range, StringRef Name);
>> -  bool mergeDeclAttribute(NamedDecl *New, InheritableAttr *Attr);
>> +  bool mergeDeclAttribute(NamedDecl *New, InheritableAttr *Attr,
>> +                          bool Override);
>> +
>> +  /// \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
>> +  };
>> 
>>   void mergeDeclAttributes(NamedDecl *New, Decl *Old,
>> -                           bool MergeDeprecation = true);
>> +                           AvailabilityMergeKind AMK = AMK_Redeclaration);
>>   void MergeTypedefNameDecl(TypedefNameDecl *New, LookupResult &OldDecls);
>>   bool MergeFunctionDecl(FunctionDecl *New, Decl *Old, Scope *S);
>>   bool MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old,
>> @@ -6383,8 +6398,7 @@
>>   /// \brief Check whether the given new method is a valid override of the
>>   /// given overridden method, and set any properties that should be inherited.
>>   void CheckObjCMethodOverride(ObjCMethodDecl *NewMethod,
>> -                               const ObjCMethodDecl *Overridden,
>> -                               bool IsImplementation);
>> +                               const ObjCMethodDecl *Overridden);
>> 
>>   /// \brief Describes the compatibility of a result type with its method.
>>   enum ResultTypeCompatibilityKind {
>> 
>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=172567&r1=172566&r2=172567&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Jan 15 16:43:08 2013
>> @@ -1822,13 +1822,14 @@
>>   return false;
>> }
>> 
>> -bool Sema::mergeDeclAttribute(NamedDecl *D, InheritableAttr *Attr) {
>> +bool Sema::mergeDeclAttribute(NamedDecl *D, InheritableAttr *Attr,
>> +                              bool Override) {
>>   InheritableAttr *NewAttr = NULL;
>>   if (AvailabilityAttr *AA = dyn_cast<AvailabilityAttr>(Attr))
>>     NewAttr = mergeAvailabilityAttr(D, AA->getRange(), AA->getPlatform(),
>>                                     AA->getIntroduced(), AA->getDeprecated(),
>>                                     AA->getObsoleted(), AA->getUnavailable(),
>> -                                    AA->getMessage());
>> +                                    AA->getMessage(), Override);
>>   else if (VisibilityAttr *VA = dyn_cast<VisibilityAttr>(Attr))
>>     NewAttr = mergeVisibilityAttr(D, VA->getRange(), VA->getVisibility());
>>   else if (DLLImportAttr *ImportA = dyn_cast<DLLImportAttr>(Attr))
>> @@ -1902,7 +1903,7 @@
>> 
>> /// mergeDeclAttributes - Copy attributes from the Old decl to the New one.
>> void Sema::mergeDeclAttributes(NamedDecl *New, Decl *Old,
>> -                               bool MergeDeprecation) {
>> +                               AvailabilityMergeKind AMK) {
>>   // attributes declared post-definition are currently ignored
>>   checkNewAttributesAfterDef(*this, New, Old);
>> 
>> @@ -1919,14 +1920,25 @@
>>          i = Old->specific_attr_begin<InheritableAttr>(),
>>          e = Old->specific_attr_end<InheritableAttr>(); 
>>        i != e; ++i) {
>> +    bool Override = false;
>>     // Ignore deprecated/unavailable/availability attributes if requested.
>> -    if (!MergeDeprecation &&
>> -        (isa<DeprecatedAttr>(*i) || 
>> -         isa<UnavailableAttr>(*i) ||
>> -         isa<AvailabilityAttr>(*i)))
>> -      continue;
>> +    if (isa<DeprecatedAttr>(*i) ||
>> +        isa<UnavailableAttr>(*i) ||
>> +        isa<AvailabilityAttr>(*i)) {
>> +      switch (AMK) {
>> +      case AMK_None:
>> +        continue;
>> +
>> +      case AMK_Redeclaration:
>> +        break;
>> +
>> +      case AMK_Override:
>> +        Override = true;
>> +        break;
>> +      }
>> +    }
>> 
>> -    if (mergeDeclAttribute(New, *i))
>> +    if (mergeDeclAttribute(New, *i, Override))
>>       foundAny = true;
>>   }
>> 
>> @@ -2475,7 +2487,7 @@
>>                                 ObjCMethodDecl *oldMethod) {
>> 
>>   // Merge the attributes, including deprecated/unavailable
>> -  mergeDeclAttributes(newMethod, oldMethod, /* mergeDeprecation */true);
>> +  mergeDeclAttributes(newMethod, oldMethod, AMK_Override);
>> 
>>   // Merge attributes from the parameters.
>>   ObjCMethodDecl::param_const_iterator oi = oldMethod->param_begin(),
>> @@ -2485,7 +2497,7 @@
>>        ni != ne && oi != oe; ++ni, ++oi)
>>     mergeParamDeclAttributes(*ni, *oi, Context);
>> 
>> -  CheckObjCMethodOverride(newMethod, oldMethod, true);
>> +  CheckObjCMethodOverride(newMethod, oldMethod);
>> }
>> 
>> /// MergeVarDeclTypes - We parsed a variable 'New' which has the same name and
>> 
>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=172567&r1=172566&r2=172567&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Jan 15 16:43:08 2013
>> @@ -2001,13 +2001,32 @@
>>   return false;
>> }
>> 
>> +/// \brief Check whether the two versions match.
>> +///
>> +/// If either version tuple is empty, then they are assumed to match. If
>> +/// \p BeforeIsOkay is true, then \p X can be less than or equal to \p Y.
>> +static bool versionsMatch(const VersionTuple &X, const VersionTuple &Y,
>> +                          bool BeforeIsOkay) {
>> +  if (X.empty() || Y.empty())
>> +    return true;
>> +
>> +  if (X == Y)
>> +    return true;
>> +
>> +  if (BeforeIsOkay && X < Y)
>> +    return true;
>> +
>> +  return false;
>> +}
>> +
>> AvailabilityAttr *Sema::mergeAvailabilityAttr(NamedDecl *D, SourceRange Range,
>>                                               IdentifierInfo *Platform,
>>                                               VersionTuple Introduced,
>>                                               VersionTuple Deprecated,
>>                                               VersionTuple Obsoleted,
>>                                               bool IsUnavailable,
>> -                                              StringRef Message) {
>> +                                              StringRef Message,
>> +                                              bool Override) {
>>   VersionTuple MergedIntroduced = Introduced;
>>   VersionTuple MergedDeprecated = Deprecated;
>>   VersionTuple MergedObsoleted = Obsoleted;
>> @@ -2033,18 +2052,47 @@
>>       VersionTuple OldDeprecated = OldAA->getDeprecated();
>>       VersionTuple OldObsoleted = OldAA->getObsoleted();
>>       bool OldIsUnavailable = OldAA->getUnavailable();
>> -      StringRef OldMessage = OldAA->getMessage();
>> 
>> -      if ((!OldIntroduced.empty() && !Introduced.empty() &&
>> -           OldIntroduced != Introduced) ||
>> -          (!OldDeprecated.empty() && !Deprecated.empty() &&
>> -           OldDeprecated != Deprecated) ||
>> -          (!OldObsoleted.empty() && !Obsoleted.empty() &&
>> -           OldObsoleted != Obsoleted) ||
>> -          (OldIsUnavailable != IsUnavailable) ||
>> -          (OldMessage != Message)) {
>> -        Diag(OldAA->getLocation(), diag::warn_mismatched_availability);
>> -        Diag(Range.getBegin(), diag::note_previous_attribute);
>> +      if (!versionsMatch(OldIntroduced, Introduced, Override) ||
>> +          !versionsMatch(Deprecated, OldDeprecated, Override) ||
>> +          !versionsMatch(Obsoleted, OldObsoleted, Override) ||
>> +          !(OldIsUnavailable == IsUnavailable ||
>> +            (Override && OldIsUnavailable && !IsUnavailable))) {
>> +        if (Override) {
>> +          int Which = -1;
>> +          VersionTuple FirstVersion;
>> +          VersionTuple SecondVersion;
>> +          if (!versionsMatch(OldIntroduced, Introduced, Override)) {
>> +            Which = 0;
>> +            FirstVersion = OldIntroduced;
>> +            SecondVersion = Introduced;
>> +          } else if (!versionsMatch(Deprecated, OldDeprecated, Override)) {
>> +            Which = 1;
>> +            FirstVersion = Deprecated;
>> +            SecondVersion = OldDeprecated;
>> +          } else if (!versionsMatch(Obsoleted, OldObsoleted, Override)) {
>> +            Which = 2;
>> +            FirstVersion = Obsoleted;
>> +            SecondVersion = OldObsoleted;
>> +          }
>> +
>> +          if (Which == -1) {
>> +            Diag(OldAA->getLocation(),
>> +                 diag::warn_mismatched_availability_override_unavail)
>> +              << AvailabilityAttr::getPrettyPlatformName(Platform->getName());
>> +          } else {
>> +            Diag(OldAA->getLocation(),
>> +                 diag::warn_mismatched_availability_override)
>> +              << Which
>> +              << AvailabilityAttr::getPrettyPlatformName(Platform->getName())
>> +              << FirstVersion.getAsString() << SecondVersion.getAsString();
>> +          }
>> +          Diag(Range.getBegin(), diag::note_overridden_method);
>> +        } else {
>> +          Diag(OldAA->getLocation(), diag::warn_mismatched_availability);
>> +          Diag(Range.getBegin(), diag::note_previous_attribute);
>> +        }
>> +
>>         Attrs.erase(Attrs.begin() + i);
>>         --e;
>>         continue;
>> @@ -2121,7 +2169,8 @@
>>                                                       Introduced.Version,
>>                                                       Deprecated.Version,
>>                                                       Obsoleted.Version,
>> -                                                      IsUnavailable, Str);
>> +                                                      IsUnavailable, Str,
>> +                                                      /*Override=*/false);
>>   if (NewAttr)
>>     D->addAttr(NewAttr);
>> }
>> 
>> Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=172567&r1=172566&r2=172567&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Tue Jan 15 16:43:08 2013
>> @@ -109,8 +109,7 @@
>> }
>> 
>> void Sema::CheckObjCMethodOverride(ObjCMethodDecl *NewMethod, 
>> -                                   const ObjCMethodDecl *Overridden,
>> -                                   bool IsImplementation) {
>> +                                   const ObjCMethodDecl *Overridden) {
>>   if (Overridden->hasRelatedResultType() && 
>>       !NewMethod->hasRelatedResultType()) {
>>     // This can only happen when the method follows a naming convention that
>> 
>> Modified: cfe/trunk/test/SemaObjC/attr-availability.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/attr-availability.m?rev=172567&r1=172566&r2=172567&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/SemaObjC/attr-availability.m (original)
>> +++ cfe/trunk/test/SemaObjC/attr-availability.m Tue Jan 15 16:43:08 2013
>> @@ -6,11 +6,24 @@
>> 
>> @interface A <P>
>> - (void)method __attribute__((availability(macosx,introduced=10.1,deprecated=10.2))); // expected-note {{method 'method' declared here}}
>> +
>> +- (void)overridden __attribute__((availability(macosx,introduced=10.3))); // expected-note{{overridden method is here}}
>> +- (void)overridden2 __attribute__((availability(macosx,introduced=10.3)));
>> +- (void)overridden3 __attribute__((availability(macosx,deprecated=10.3)));
>> +- (void)overridden4 __attribute__((availability(macosx,deprecated=10.3))); // expected-note{{overridden method is here}}
>> +- (void)overridden5 __attribute__((availability(macosx,unavailable))); // expected-note{{overridden method is here}}
>> +- (void)overridden6 __attribute__((availability(macosx,introduced=10.3)));
>> @end
>> 
>> // rdar://11475360
>> @interface B : A
>> - (void)method; // expected-note {{method 'method' declared here}}
>> +- (void)overridden __attribute__((availability(macosx,introduced=10.4))); // expected-warning{{overriding method introduced after overridden method on OS X (10.4 vs. 10.3)}}
>> +- (void)overridden2 __attribute__((availability(macosx,introduced=10.2)));
>> +- (void)overridden3 __attribute__((availability(macosx,deprecated=10.4)));
>> +- (void)overridden4 __attribute__((availability(macosx,deprecated=10.2))); // expected-warning{{overriding method deprecated before overridden method on OS X (10.3 vs. 10.2)}}
>> +- (void)overridden5 __attribute__((availability(macosx,introduced=10.3))); // expected-warning{{overriding method cannot be unavailable on OS X when its overridden method is available}}
>> +- (void)overridden6 __attribute__((availability(macosx,unavailable)));
>> @end
>> 
>> void f(A *a, B *b) {
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130115/d57e6c11/attachment.html>


More information about the cfe-commits mailing list