From 96e6e5042b45334a957d1670db2179ee569b3ae1 Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Tue, 14 Apr 2020 14:23:33 -0700 Subject: [PATCH 1/2] Fix @available(macCatalyst N, iOS N+M) bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, availability checking computed a declaration’s availability as the intersection of all @available attributes with active platforms. This meant that Swift would not allow you to use an API that was available earlier in a child platform than in its parent until it was also available in the parent platform. That was incorrect. This PR corrects availability checking to find the most specific @available attribute with an introduced version and use that instead. Fixes rdar://60892534. --- lib/AST/Availability.cpp | 24 +++++++++---------- test/attr/attr_availability_maccatalyst.swift | 17 +++++++++++++ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/lib/AST/Availability.cpp b/lib/AST/Availability.cpp index 4512b15047735..38f788622bbb8 100644 --- a/lib/AST/Availability.cpp +++ b/lib/AST/Availability.cpp @@ -134,7 +134,7 @@ void AvailabilityInference::applyInferredAvailableAttrs( Optional AvailabilityInference::annotatedAvailableRange(const Decl *D, ASTContext &Ctx) { - Optional AnnotatedRange; + const AvailableAttr *bestAvailAttr = nullptr; for (auto Attr : D->getAttrs()) { auto *AvailAttr = dyn_cast(Attr); @@ -145,21 +145,19 @@ AvailabilityInference::annotatedAvailableRange(const Decl *D, ASTContext &Ctx) { continue; } - AvailabilityContext AttrRange{ - VersionRange::allGTE(AvailAttr->Introduced.getValue())}; - - // If we have multiple introduction versions, we will conservatively - // assume the worst case scenario. We may want to be more precise here - // in the future or emit a diagnostic. - - if (AnnotatedRange.hasValue()) { - AnnotatedRange.getValue().intersectWith(AttrRange); - } else { - AnnotatedRange = AttrRange; + // Okay, we have a candidate, but is it better than one we already found? + if (!bestAvailAttr || + inheritsAvailabilityFromPlatform(AvailAttr->Platform, + bestAvailAttr->Platform)) { + bestAvailAttr = AvailAttr; } } - return AnnotatedRange; + if (!bestAvailAttr) + return None; + + return AvailabilityContext{ + VersionRange::allGTE(bestAvailAttr->Introduced.getValue())}; } AvailabilityContext AvailabilityInference::availableRange(const Decl *D, diff --git a/test/attr/attr_availability_maccatalyst.swift b/test/attr/attr_availability_maccatalyst.swift index 3c3bf676f1c8b..07fd5229471d1 100644 --- a/test/attr/attr_availability_maccatalyst.swift +++ b/test/attr/attr_availability_maccatalyst.swift @@ -48,12 +48,18 @@ deprecatedOnIOSButNotMacCatalyst() // no-warning func introducedLaterOnMacCatalyst() { } +@available(iOS 57.0, macCatalyst 56.0, *) +func introducedLaterOnIOS() { +} + // expected-note@+1 *{{add @available attribute to enclosing global function}} func testPoundAvailable() { if #available(macCatalyst 55.0, *) { introducedLaterOnMacCatalyst() // expected-error {{'introducedLaterOnMacCatalyst()' is only available in Mac Catalyst 56.0 or newer}} // expected-note@-1 {{add 'if #available' version check}} + introducedLaterOnIOS() // expected-error {{'introducedLaterOnIOS()' is only available in Mac Catalyst 56.0 or newer}} + // expected-note@-1 {{add 'if #available' version check}} } // macCatalyst should win over iOS when present @@ -61,10 +67,18 @@ func testPoundAvailable() { if #available(iOS 56.0, macCatalyst 55.0, *) { introducedLaterOnMacCatalyst() // expected-error {{'introducedLaterOnMacCatalyst()' is only available in Mac Catalyst 56.0 or newer}} // expected-note@-1 {{add 'if #available' version check}} + introducedLaterOnIOS() // expected-error {{'introducedLaterOnIOS()' is only available in Mac Catalyst 56.0 or newer}} + // expected-note@-1 {{add 'if #available' version check}} } if #available(iOS 55.0, macCatalyst 56.0, *) { introducedLaterOnMacCatalyst() // no-warning + introducedLaterOnIOS() // no-error + } + + if #available(iOS 57.0, macCatalyst 56.0, *) { + introducedLaterOnMacCatalyst() // no-warning + introducedLaterOnIOS() // no-error } // iOS availability should be inherited when macCatalyst is not present @@ -72,10 +86,13 @@ func testPoundAvailable() { if #available(iOS 55.0, *) { introducedLaterOnMacCatalyst() // expected-error {{'introducedLaterOnMacCatalyst()' is only available in Mac Catalyst 56.0 or newer}} // expected-note@-1 {{add 'if #available' version check}} + introducedLaterOnIOS() // expected-error {{'introducedLaterOnIOS()' is only available in Mac Catalyst 56.0 or newer}} + // expected-note@-1 {{add 'if #available' version check}} } if #available(iOS 56.0, *) { introducedLaterOnMacCatalyst() // no-warning + introducedLaterOnIOS() // no-error } // macOS availability doesn't count on macCatalyst for Swift. From 05736d14b872ae9754e2ec091633eabd49d42cdc Mon Sep 17 00:00:00 2001 From: Brent Royal-Gordon Date: Fri, 17 Apr 2020 02:31:25 -0700 Subject: [PATCH 2/2] Handle multiple @available attributes correctly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit broke a promise made by the implementation it replaced: if there were two or more @available(introduced:) attributes for the same platform, the greatest version wins. This commit restores that property while still completely overriding the parent platform’s availability with a child platform’s. --- lib/AST/Availability.cpp | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/lib/AST/Availability.cpp b/lib/AST/Availability.cpp index 38f788622bbb8..7eacce69db72e 100644 --- a/lib/AST/Availability.cpp +++ b/lib/AST/Availability.cpp @@ -132,6 +132,26 @@ void AvailabilityInference::applyInferredAvailableAttrs( } } +/// Returns true if the introduced version in \p newAttr should be used instead +/// of the introduced version in \p prevAttr when both are attached to the same +/// declaration and refer to the active platform. +static bool isBetterThan(const AvailableAttr *newAttr, + const AvailableAttr *prevAttr) { + assert(newAttr); + + // If there is no prevAttr, newAttr of course wins. + if (!prevAttr) + return true; + + // If they belong to the same platform, the one that introduces later wins. + if (prevAttr->Platform == newAttr->Platform) + return prevAttr->Introduced.getValue() < newAttr->Introduced.getValue(); + + // If the new attribute's platform inherits from the old one, it wins. + return inheritsAvailabilityFromPlatform(newAttr->Platform, + prevAttr->Platform); +} + Optional AvailabilityInference::annotatedAvailableRange(const Decl *D, ASTContext &Ctx) { const AvailableAttr *bestAvailAttr = nullptr; @@ -145,12 +165,8 @@ AvailabilityInference::annotatedAvailableRange(const Decl *D, ASTContext &Ctx) { continue; } - // Okay, we have a candidate, but is it better than one we already found? - if (!bestAvailAttr || - inheritsAvailabilityFromPlatform(AvailAttr->Platform, - bestAvailAttr->Platform)) { + if (isBetterThan(AvailAttr, bestAvailAttr)) bestAvailAttr = AvailAttr; - } } if (!bestAvailAttr)