Merge pull request swiftlang#36327 from slavapestov/fix-storage-availability

Fix availability checking for stored properties and enum cases with associated values

Author: slavapestov

include/swift/AST/DiagnosticsSema.def

@@ -5184,6 +5184,10 @@ ERROR(availability_stored_property_no_potential,
       none, "stored properties cannot be marked potentially unavailable with "
       "'@available'", ())
 
+ERROR(availability_enum_element_no_potential,
+      none, "enum cases with associated values cannot be marked potentially unavailable with "
+      "'@available'", ())
+
 ERROR(availability_protocol_requires_version,
       none, "protocol %0 requires %1 to be available in %2 %3 and newer",
       (Identifier, DeclName, StringRef, llvm::VersionTuple))

lib/Sema/TypeCheckAttr.cpp

@@ -1471,12 +1471,6 @@ void AttributeChecker::visitAvailableAttr(AvailableAttr *attr) {
 
   SourceLoc attrLoc = attr->getLocation();
 
-  Optional<Diag<>> MaybeNotAllowed =
-      TypeChecker::diagnosticIfDeclCannotBePotentiallyUnavailable(D);
-  if (MaybeNotAllowed.hasValue()) {
-    diagnose(attrLoc, MaybeNotAllowed.getValue());
-  }
-
   // 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
@@ -1494,16 +1488,27 @@ void AttributeChecker::visitAvailableAttr(AvailableAttr *attr) {
     EnclosingDecl = getEnclosingDeclForDecl(EnclosingDecl);
   }
 
-  if (!EnclosingDecl)
-    return;
-
   AvailabilityContext AttrRange{
       VersionRange::allGTE(attr->Introduced.getValue())};
 
-  if (!AttrRange.isContainedIn(EnclosingAnnotatedRange.getValue())) {
-    diagnose(attr->getLocation(), diag::availability_decl_more_than_enclosing);
-    diagnose(EnclosingDecl->getLoc(),
-             diag::availability_decl_more_than_enclosing_enclosing_here);
+  if (EnclosingDecl) {
+    if (!AttrRange.isContainedIn(EnclosingAnnotatedRange.getValue())) {
+      diagnose(attr->getLocation(), diag::availability_decl_more_than_enclosing);
+      diagnose(EnclosingDecl->getLoc(),
+               diag::availability_decl_more_than_enclosing_enclosing_here);
+    }
+  }
+
+  Optional<Diag<>> MaybeNotAllowed =
+      TypeChecker::diagnosticIfDeclCannotBePotentiallyUnavailable(D);
+  if (MaybeNotAllowed.hasValue()) {
+    AvailabilityContext DeploymentRange
+        = AvailabilityContext::forDeploymentTarget(Ctx);
+    if (EnclosingAnnotatedRange.hasValue())
+      DeploymentRange.intersectWith(*EnclosingAnnotatedRange);
+
+    if (!DeploymentRange.isContainedIn(AttrRange))
+      diagnose(attrLoc, MaybeNotAllowed.getValue());
   }
 }
 
@@ -3399,31 +3404,26 @@ Type TypeChecker::checkReferenceOwnershipAttr(VarDecl *var, Type type,
 
 Optional<Diag<>>
 TypeChecker::diagnosticIfDeclCannotBePotentiallyUnavailable(const Decl *D) {
-  DeclContext *DC = D->getDeclContext();
-  // Do not permit potential availability of script-mode global variables;
-  // their initializer expression is not lazily evaluated, so this would
-  // not be safe.
-  if (isa<VarDecl>(D) && DC->isModuleScopeContext() &&
-      DC->getParentSourceFile()->isScriptMode()) {
-    return diag::availability_global_script_no_potential;
-  }
-
-  // For now, we don't allow stored properties to be potentially unavailable.
-  // We will want to support these eventually, but we haven't figured out how
-  // this will interact with Definite Initialization, deinitializers and
-  // resilience yet.
   if (auto *VD = dyn_cast<VarDecl>(D)) {
+    if (!VD->hasStorage())
+      return None;
+
+    // Do not permit potential availability of script-mode global variables;
+    // their initializer expression is not lazily evaluated, so this would
+    // not be safe.
+    if (VD->isTopLevelGlobal())
+      return diag::availability_global_script_no_potential;
+
     // Globals and statics are lazily initialized, so they are safe
-    // for potential unavailability. Note that if D is a global in script
-    // mode (which are not lazy) then we will already have returned
-    // a diagnosis above.
-    bool lazilyInitializedStored = VD->isStatic() ||
-                                   VD->getAttrs().hasAttribute<LazyAttr>() ||
-                                   DC->isModuleScopeContext();
-
-    if (VD->hasStorage() && !lazilyInitializedStored) {
+    // for potential unavailability.
+    if (!VD->isStatic() && !VD->getDeclContext()->isModuleScopeContext())
       return diag::availability_stored_property_no_potential;
-    }
+
+  } else if (auto *EED = dyn_cast<EnumElementDecl>(D)) {
+    // An enum element with an associated value cannot be potentially
+    // unavailable.
+    if (EED->hasAssociatedValues())
+      return diag::availability_enum_element_no_potential;
   }
 
   return None;

test/Sema/availability_stored.swift

@@ -0,0 +1,60 @@
+// RUN: %target-typecheck-verify-swift
+
+// Code should type check with a new enough deployment target:
+// RUN: %target-swift-frontend -typecheck %s -target x86_64-apple-macos50
+
+// REQUIRES: OS=macosx
+
+@available(macOS 50, *)
+struct NewStruct {}
+
+@available(macOS 50, *)
+struct GoodReferenceStruct {
+  var x: NewStruct
+}
+
+@available(macOS 50, *)
+struct GoodNestedReferenceStruct {
+  struct Inner {
+    var x: NewStruct
+  }
+}
+
+struct BadReferenceStruct1 {
+  // expected-error@+1 {{stored properties cannot be marked potentially unavailable with '@available'}}
+  @available(macOS 50, *)
+  var x: NewStruct
+}
+
+@available(macOS 40, *)
+struct BadReferenceStruct2 {
+  // expected-error@+1 {{stored properties cannot be marked potentially unavailable with '@available'}}
+  @available(macOS 50, *)
+  var x: NewStruct
+}
+
+// The same behavior should hold for enum elements with payloads.
+@available(macOS 50, *)
+enum GoodReferenceEnum {
+  case x(NewStruct)
+}
+
+@available(macOS 50, *)
+enum GoodNestedReferenceEnum {
+  enum Inner {
+    case x(NewStruct)
+  }
+}
+
+enum BadReferenceEnum1 {
+  // expected-error@+1 {{enum cases with associated values cannot be marked potentially unavailable with '@available'}}
+  @available(macOS 50, *)
+  case x(NewStruct)
+}
+
+@available(macOS 40, *)
+enum BadReferenceEnum2 {
+  // expected-error@+1 {{enum cases with associated values cannot be marked potentially unavailable with '@available'}}
+  @available(macOS 50, *)
+  case x(NewStruct)
+}

test/Sema/availability_versions.swift

@@ -309,7 +309,6 @@ class SubOfClassWithUnavailableInitializer : SuperWithWithUnavailableInitializer
 class ClassWithUnavailableProperties {
     // expected-note@-1 4{{add @available attribute to enclosing class}}
 
-  @available(OSX, introduced: 10.9) // expected-error {{stored properties cannot be marked potentially unavailable with '@available'}}
   var nonLazyAvailableOn10_9Stored: Int = 9
 
   @available(OSX, introduced: 10.51) // expected-error {{stored properties cannot be marked potentially unavailable with '@available'}}
@@ -538,17 +537,17 @@ enum CompassPoint {
 
   case WithAvailableByEnumPayload(p : EnumIntroducedOn10_51)
 
+  // expected-error@+1 {{enum cases with associated values cannot be marked potentially unavailable with '@available'}}
   @available(OSX, introduced: 10.52)
   case WithAvailableByEnumElementPayload(p : EnumIntroducedOn10_52)
 
+  // expected-error@+1 2{{enum cases with associated values cannot be marked potentially unavailable with '@available'}}
   @available(OSX, introduced: 10.52)
   case WithAvailableByEnumElementPayload1(p : EnumIntroducedOn10_52), WithAvailableByEnumElementPayload2(p : EnumIntroducedOn10_52)
 
   case WithUnavailablePayload(p : EnumIntroducedOn10_52) // expected-error {{'EnumIntroducedOn10_52' is only available in macOS 10.52 or newer}}
-      // expected-note@-1 {{add @available attribute to enclosing case}}
 
-    case WithUnavailablePayload1(p : EnumIntroducedOn10_52), WithUnavailablePayload2(p : EnumIntroducedOn10_52) // expected-error 2{{'EnumIntroducedOn10_52' is only available in macOS 10.52 or newer}}
-      // expected-note@-1 2{{add @available attribute to enclosing case}}
+  case WithUnavailablePayload1(p : EnumIntroducedOn10_52), WithUnavailablePayload2(p : EnumIntroducedOn10_52) // expected-error 2{{'EnumIntroducedOn10_52' is only available in macOS 10.52 or newer}}
 }
 
 @available(OSX, introduced: 10.52)
@@ -1337,11 +1336,9 @@ enum EnumForFixit {
       // expected-note@-1 2{{add @available attribute to enclosing enum}} {{1-1=@available(macOS 10.51, *)\n}}
   case CaseWithUnavailablePayload(p: ClassAvailableOn10_51)
       // expected-error@-1 {{'ClassAvailableOn10_51' is only available in macOS 10.51 or newer}}
-      // expected-note@-2 {{add @available attribute to enclosing case}} {{3-3=@available(macOS 10.51, *)\n  }}
 
   case CaseWithUnavailablePayload2(p: ClassAvailableOn10_51), WithoutPayload
       // expected-error@-1 {{'ClassAvailableOn10_51' is only available in macOS 10.51 or newer}}
-      // expected-note@-2 {{add @available attribute to enclosing case}} {{3-3=@available(macOS 10.51, *)\n  }}
 
 }