On Jan 15, 2013, at 3:25 PM, Jordan Rose <[email protected]> wrote:
> > On Jan 15, 2013, at 14:43 , Douglas Gregor <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
