Sema: Diagnose @_spi_available on declarations that cannot be unavailable.

The attribute makes the declaration unavailable from the perspective of clients
of the module's public interface and was creating a loophole that admitted
inappropriate unavailability.

Author: tshortli

lib/Sema/TypeCheckAttr.cpp

@@ -2338,6 +2338,13 @@ void AttributeChecker::visitAvailableAttr(AvailableAttr *parsedAttr) {
       diagnoseAndRemoveAttr(parsedAttr, diag::spi_available_malformed);
       return;
     }
+
+    if (auto diag =
+            TypeChecker::diagnosticIfDeclCannotBeUnavailable(D, *attr)) {
+      diagnoseAndRemoveAttr(const_cast<AvailableAttr *>(attr->getParsedAttr()),
+                            *diag);
+      return;
+    }
   }
 
   if (attr->isNoAsync()) {
@@ -5096,9 +5103,9 @@ void AttributeChecker::checkAvailableAttrs(ArrayRef<AvailableAttr *> attrs) {
   // declaration that is allowed to be unavailable, diagnose.
   if (availabilityConstraint->isUnavailable()) {
     auto attr = availabilityConstraint->getAttr();
-    if (auto diag = TypeChecker::diagnosticIfDeclCannotBeUnavailable(D)) {
+    if (auto diag = TypeChecker::diagnosticIfDeclCannotBeUnavailable(D, attr)) {
       diagnoseAndRemoveAttr(const_cast<AvailableAttr *>(attr.getParsedAttr()),
-                            diag.value());
+                            *diag);
       return;
     }
   }
@@ -5443,7 +5450,8 @@ TypeChecker::diagnosticIfDeclCannotBePotentiallyUnavailable(const Decl *D) {
 }
 
 std::optional<Diagnostic>
-TypeChecker::diagnosticIfDeclCannotBeUnavailable(const Decl *D) {
+TypeChecker::diagnosticIfDeclCannotBeUnavailable(const Decl *D,
+                                                 SemanticAvailableAttr attr) {
   auto parentIsUnavailable = [](const Decl *D) -> bool {
     if (auto *parent =
             AvailabilityInference::parentDeclForInferredAvailability(D)) {
@@ -5479,7 +5487,7 @@ TypeChecker::diagnosticIfDeclCannotBeUnavailable(const Decl *D) {
 
         // Be lenient in interfaces to accomodate @_spi_available, which has
         // been accepted historically.
-        if (DC->isInSwiftinterface())
+        if (attr.isSPI() || DC->isInSwiftinterface())
           diag.setBehaviorLimit(DiagnosticBehavior::Warning);
         return diag;
       }
@@ -5493,7 +5501,13 @@ TypeChecker::diagnosticIfDeclCannotBeUnavailable(const Decl *D) {
     if (parentIsUnavailable(D))
       return std::nullopt;
 
-    // Be lenient in interfaces to accomodate @_spi_available.
+    // @_spi_available does not make the declaration unavailable from the
+    // perspective of the owning module, which is what matters.
+    if (attr.isSPI())
+      return std::nullopt;
+
+    // An unavailable property with storage encountered in a swiftinterface
+    // might have been declared @_spi_available in source.
     if (D->getDeclContext()->isInSwiftinterface())
       return std::nullopt;
 

lib/Sema/TypeChecker.h

@@ -20,6 +20,7 @@
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/AccessScope.h"
 #include "swift/AST/AnyFunctionRef.h"
+#include "swift/AST/Attr.h"
 #include "swift/AST/AvailabilityRange.h"
 #include "swift/AST/AvailabilityScope.h"
 #include "swift/AST/DiagnosticsSema.h"
@@ -1065,7 +1066,8 @@ diagnosticIfDeclCannotBePotentiallyUnavailable(const Decl *D);
 /// Returns a diagnostic indicating why the declaration cannot be annotated
 /// with an @available() attribute indicating it is unavailable or None if this
 /// is allowed.
-std::optional<Diagnostic> diagnosticIfDeclCannotBeUnavailable(const Decl *D);
+std::optional<Diagnostic>
+diagnosticIfDeclCannotBeUnavailable(const Decl *D, SemanticAvailableAttr attr);
 
 /// Checks whether the required range of versions of the compilation's target
 /// platform are available at the given `SourceRange`. If not, `Diagnose` is

test/Sema/availability_accessors.swift

@@ -569,5 +569,4 @@ struct BadAccessorAvailability<T> {
     @available(swift, introduced: 99) // expected-error {{didSet observer for property cannot be marked unavailable with '@available'}}
     didSet { }
   }
-
 }

test/Sema/availability_deinit.swift

@@ -31,6 +31,11 @@ class DeinitUnavailableMacOS {
   deinit {}
 }
 
+class DeinitSPIAvailableMacOS {
+  @_spi_available(macOS, introduced: 50) // expected-error {{deinitializer cannot be marked unavailable with '@available'}}
+  deinit {}
+}
+
 class AvailableAtDeploymentTargetDeinit {
   @available(macOS 50, *)
   deinit {}

test/Sema/availability_stored_unavailable.swift

@@ -5,13 +5,21 @@
 @available(macOS, unavailable)
 struct UnavailableMacOSStruct {} // expected-note 2 {{'UnavailableMacOSStruct' has been explicitly marked unavailable here}}
 
+@available(iOS, introduced: 8.0)
+@_spi_available(macOS, introduced: 10.9)
+public struct SPIAvailableMacOSStruct {}
+
 @available(*, unavailable)
 public struct UniversallyUnavailableStruct {} // expected-note {{'UniversallyUnavailableStruct' has been explicitly marked unavailable here}}
 
 // Ok, initialization of globals is lazy and boxed.
 @available(macOS, unavailable)
 var unavailableMacOSGlobal: UnavailableMacOSStruct = .init()
 
+@available(iOS, introduced: 8.0)
+@_spi_available(macOS, introduced: 10.9)
+var spiAvailableMacOSGlobal: SPIAvailableMacOSStruct = .init()
+
 @available(*, unavailable)
 var universallyUnavailableGlobal: UniversallyUnavailableStruct = .init()
 
@@ -22,6 +30,10 @@ struct GoodAvailableStruct {
     get { UnavailableMacOSStruct() }
   }
 
+  // Ok, storage is available to implementation at runtime.
+  @_spi_available(macOS, introduced: 10.9)
+  var spiAvailableMacOS: SPIAvailableMacOSStruct
+
   // Ok, initialization of static vars is lazy and boxed.
   @available(macOS, unavailable)
   static var staticUnavailableMacOS: UnavailableMacOSStruct = .init()
@@ -75,4 +87,7 @@ struct BadStruct {
 enum GoodAvailableEnum {
   @available(macOS, unavailable)
   case unavailableMacOS(UnavailableMacOSStruct)
+
+  @_spi_available(macOS, introduced: 10.9)
+  case spiAvailableMacOS(SPIAvailableMacOSStruct)
 }

test/Sema/availability_versions.swift

@@ -1934,4 +1934,13 @@ struct PropertyObservers {
     @available(macOS, obsoleted: 10.9) // expected-error {{didSet observer for property cannot be marked unavailable with '@available'}}
     didSet { }
   }
+
+  var hasSPIAvailableObservers: Int {
+    @_spi_available(macOS, introduced: 10.9) // expected-error {{willSet observer for property cannot be marked unavailable with '@available'}}
+    willSet { }
+
+    @_spi_available(macOS, introduced: 10.9) // expected-error {{didSet observer for property cannot be marked unavailable with '@available'}}
+    didSet { }
+  }
+
 }

test/decl/protocol/req/unavailable.swift

@@ -15,8 +15,13 @@ class Foo : Proto {
 // Reject protocols with 'unavailable' requirements
 // if a protocol is not marked @objc.
 protocol NonObjCProto {
-  @available(*,unavailable) func bad() // expected-error {{protocol members can only be marked unavailable in an @objc protocol}} expected-note {{protocol requires function 'bad()'}}
+  @available(*,unavailable) // expected-error {{protocol members can only be marked unavailable in an @objc protocol}}
+  func bad() // expected-note {{protocol requires function 'bad()'}}
+
   func good()
+
+  @_spi_available(macOS, introduced: 10.9) // expected-warning {{protocol members can only be marked unavailable in an @objc protocol}}
+  func kindaBad() // expected-note {{protocol requires function 'kindaBad()'}}
 }
 
 class Bar : NonObjCProto { // expected-error {{type 'Bar' does not conform to protocol 'NonObjCProto'}} expected-note {{add stubs for conformance}}
@@ -115,4 +120,7 @@ protocol UnavailableAssoc {
 
   @available(swift, obsoleted: 99)
   associatedtype A5
+
+  @_spi_available(macOS, introduced: 11) // expected-error {{associated type cannot be marked unavailable with '@available'}}
+  associatedtype A6
 }