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.

Author: tshortli

include/swift/AST/Decl.h

@@ -1101,17 +1101,21 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
   /// 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<std::pair<const AvailableAttr *, const Decl *>>
   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<std::pair<const AvailableAttr *, const Decl *>>
   getSemanticUnavailableAttr() const;
 

lib/Sema/TypeCheckAttr.cpp

@@ -1877,56 +1877,54 @@ void AttributeChecker::visitAvailableAttr(AvailableAttr *attr) {
 
   SourceLoc attrLoc = attr->getLocation();
 
+  AvailabilityContext AttrRange{
+      VersionRange::allGTE(attr->Introduced.value())};
+
   // Find the innermost enclosing declaration with an availability
   // range annotation and ensure that this attribute's available version range
   // is fully contained within that declaration's range. If there is no such
   // enclosing declaration, then there is nothing to check.
   Optional<AvailabilityContext> 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<ExtensionDecl>(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());
+      }
     }
   }
 

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}}

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() {}
 }