From abf4d127a4b51ea7ed1da4ad5afcb494267af1f4 Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Fri, 6 Jan 2023 14:11:24 -0800 Subject: [PATCH] Sema: Improve diagnostics for decls more available than their containers. Adopt new request-based utilities for looking up the enclosing declaration's availability when type checking an `@available` attribute. This consolidates implementations of the lookup and improves diagnostics by catching more cases where declarations are more available than their containers. --- include/swift/AST/Decl.h | 14 +++-- lib/Sema/TypeCheckAttr.cpp | 67 +++++++++++------------- test/attr/attr_availability_osx.swift | 9 +++- test/attr/attr_inlinable_available.swift | 4 +- 4 files changed, 50 insertions(+), 44 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 5b13910c4dd..92568e28155 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -1101,17 +1101,21 @@ public: /// Retrieve the @available attribute that provides the OS version range that /// this declaration is available in. /// - /// The attribute may come from another declaration, since availability - /// could be inherited from a parent declaration. + /// This attribute may come from an enclosing decl since availability is + /// inherited. The second member of the returned pair is the decl that owns + /// the attribute. Optional> getSemanticAvailableRangeAttr() const; /// Retrieve the @available attribute that makes this declaration unavailable, /// if any. /// - /// The attribute may come from another declaration, since unavailability - /// could be inherited from a parent declaration. This is a broader notion of - /// unavailability than is checked by \c AvailableAttr::isUnavailable. + /// This attribute may come from an enclosing decl since availability is + /// inherited. The second member of the returned pair is the decl that owns + /// the attribute. + /// + /// Note that this notion of unavailability is broader than that which is + /// checked by \c AvailableAttr::isUnavailable. Optional> getSemanticUnavailableAttr() const; diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index f1d62fb288b..69e0dd399b1 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -1882,51 +1882,48 @@ void AttributeChecker::visitAvailableAttr(AvailableAttr *attr) { // is fully contained within that declaration's range. If there is no such // enclosing declaration, then there is nothing to check. Optional EnclosingAnnotatedRange; - bool EnclosingDeclIsUnavailable = false; - Decl *EnclosingDecl = getEnclosingDeclForDecl(D); - - while (EnclosingDecl) { - if (EnclosingDecl->getAttrs().getUnavailable(Ctx)) { - EnclosingDeclIsUnavailable = true; - break; - } - - EnclosingAnnotatedRange = - AvailabilityInference::annotatedAvailableRange(EnclosingDecl, Ctx); - - if (EnclosingAnnotatedRange.has_value()) - break; - - EnclosingDecl = getEnclosingDeclForDecl(EnclosingDecl); - } - AvailabilityContext AttrRange{ VersionRange::allGTE(attr->Introduced.value())}; - if (EnclosingDecl) { - if (EnclosingDeclIsUnavailable) { + if (auto *parent = getEnclosingDeclForDecl(D)) { + if (auto enclosingUnavailable = parent->getSemanticUnavailableAttr()) { if (!AttrRange.isKnownUnreachable()) { - diagnose(D->isImplicit() ? EnclosingDecl->getLoc() + const Decl *enclosingDecl = enclosingUnavailable.value().second; + diagnose(D->isImplicit() ? enclosingDecl->getLoc() : attr->getLocation(), diag::availability_decl_more_than_unavailable_enclosing, D->getDescriptiveKind()); - diagnose(EnclosingDecl->getLoc(), + diagnose(parent->getLoc(), diag::availability_decl_more_than_unavailable_enclosing_here); } - } else if (!AttrRange.isContainedIn(EnclosingAnnotatedRange.value())) { - diagnose(D->isImplicit() ? EnclosingDecl->getLoc() : attr->getLocation(), - diag::availability_decl_more_than_enclosing, - D->getDescriptiveKind()); - if (D->isImplicit()) - diagnose(EnclosingDecl->getLoc(), - diag::availability_implicit_decl_here, - D->getDescriptiveKind(), + } else if (auto enclosingAvailable = + parent->getSemanticAvailableRangeAttr()) { + const AvailableAttr *enclosingAttr = enclosingAvailable.value().first; + const Decl *enclosingDecl = enclosingAvailable.value().second; + EnclosingAnnotatedRange.emplace( + VersionRange::allGTE(enclosingAttr->Introduced.value())); + if (!AttrRange.isContainedIn(*EnclosingAnnotatedRange)) { + // Members of extensions of nominal types with available ranges were + // not diagnosed previously, so only emit a warning in that case. + auto limit = (enclosingDecl != parent && isa(parent)) + ? DiagnosticBehavior::Warning + : DiagnosticBehavior::Unspecified; + diagnose(D->isImplicit() ? enclosingDecl->getLoc() + : attr->getLocation(), + diag::availability_decl_more_than_enclosing, + D->getDescriptiveKind()) + .limitBehavior(limit); + if (D->isImplicit()) + diagnose(enclosingDecl->getLoc(), + diag::availability_implicit_decl_here, + D->getDescriptiveKind(), + prettyPlatformString(targetPlatform(Ctx.LangOpts)), + AttrRange.getOSVersion().getLowerEndpoint()); + diagnose(enclosingDecl->getLoc(), + diag::availability_decl_more_than_enclosing_here, prettyPlatformString(targetPlatform(Ctx.LangOpts)), - AttrRange.getOSVersion().getLowerEndpoint()); - diagnose(EnclosingDecl->getLoc(), - diag::availability_decl_more_than_enclosing_here, - prettyPlatformString(targetPlatform(Ctx.LangOpts)), - EnclosingAnnotatedRange->getOSVersion().getLowerEndpoint()); + EnclosingAnnotatedRange->getOSVersion().getLowerEndpoint()); + } } } diff --git a/test/attr/attr_availability_osx.swift b/test/attr/attr_availability_osx.swift index cee922a7c7c..c80e90a626f 100644 --- a/test/attr/attr_availability_osx.swift +++ b/test/attr/attr_availability_osx.swift @@ -80,8 +80,8 @@ func doSomethingDeprecatedOniOS() { } doSomethingDeprecatedOniOS() // okay - -struct TestStruct {} +@available(macOS 10.10, *) +struct TestStruct {} // expected-note {{enclosing scope requires availability of macOS 10.10 or newer}} @available(macOS 10.10, *) extension TestStruct { // expected-note {{enclosing scope requires availability of macOS 10.10 or newer}} @@ -104,6 +104,11 @@ extension TestStruct { // expected-note {{enclosing scope requires availability func doDeprecatedThing() {} } +extension TestStruct { + @available(macOS 10.9, *) // expected-warning {{instance method cannot be more available than enclosing scope}} + func doFifthThing() {} +} + @available(macOS 10.11, *) func testMemberAvailability() { TestStruct().doTheThing() // expected-error {{'doTheThing()' is unavailable}} diff --git a/test/attr/attr_inlinable_available.swift b/test/attr/attr_inlinable_available.swift index 792ef34c632..63e57545726 100644 --- a/test/attr/attr_inlinable_available.swift +++ b/test/attr/attr_inlinable_available.swift @@ -55,7 +55,7 @@ public struct AtInliningTarget { } @available(macOS 10.14.5, *) -public struct BetweenTargets { +public struct BetweenTargets { // expected-note {{enclosing scope requires availability of macOS 10.14.5 or newer}} @usableFromInline internal init() {} } @@ -1102,7 +1102,7 @@ extension BetweenTargets { } extension BetweenTargets { - @available(macOS 10.10, *) + @available(macOS 10.10, *) // expected-warning {{instance method cannot be more available than enclosing scope}} func excessivelyAvailableInternalFuncInExtension() {} }