Sema: Diagnose override availability for custom domains.

Resolves rdar://156159486.

include/swift/AST/DiagnosticsSema.def

@@ -3519,13 +3519,17 @@ ERROR(multiple_override,none,
 NOTE(multiple_override_prev,none,
      "%0 previously overridden here", (DeclName))
 
-ERROR(override_unavailable, none,
+ERROR(cannot_override_unavailable, none,
       "cannot override %base0 which has been marked unavailable%select{|: %1}1",
       (ValueDecl *, StringRef))
 NOTE(suggest_removing_override, none,
      "remove 'override' modifier to declare a new %0",
      (DeclBaseName))
 
+ERROR(override_unavailable, none,
+      "cannot override %base0 with a declaration that is marked unavailable",
+      (ValueDecl *))
+
 ERROR(override_less_available,none,
       "overriding %base0 must be as available as declaration it overrides",
       (ValueDecl *))

lib/Sema/TypeCheckAvailability.cpp

@@ -1630,7 +1630,7 @@ void swift::diagnoseOverrideOfUnavailableDecl(ValueDecl *override,
   auto &diags = ctx.Diags;
   if (attr.getRename().empty()) {
     EncodedDiagnosticMessage EncodedMessage(attr.getMessage());
-    diags.diagnose(override, diag::override_unavailable,
+    diags.diagnose(override, diag::cannot_override_unavailable,
                    override, EncodedMessage.Message);
 
     diags.diagnose(base, diag::availability_marked_unavailable, base);

lib/Sema/TypeCheckDeclOverride.cpp

@@ -22,7 +22,7 @@
 #include "TypeCheckUnsafe.h"
 #include "TypeChecker.h"
 #include "swift/AST/ASTVisitor.h"
-#include "swift/AST/AvailabilityInference.h"
+#include "swift/AST/AvailabilityConstraint.h"
 #include "swift/AST/AvailabilityRange.h"
 #include "swift/AST/Decl.h"
 #include "swift/AST/GenericEnvironment.h"
@@ -1810,159 +1810,97 @@ OverrideRequiresKeyword swift::overrideRequiresKeyword(ValueDecl *overridden) {
   return OverrideRequiresKeyword::Always;
 }
 
-/// Returns true if the availability of the overriding declaration
+enum class OverrideAvailability {
-/// makes it a safe override, given the availability of the base declaration.
-static bool isAvailabilitySafeForOverride(ValueDecl *override,
-                                          ValueDecl *base) {
-  // API availability ranges are contravariant: make sure the version range
-  // of an overridden declaration is fully contained in the range of the
-  // overriding declaration.
-  AvailabilityRange overrideInfo =
-      AvailabilityInference::availableRange(override);
-  AvailabilityRange baseInfo = AvailabilityInference::availableRange(base);
-
-  if (baseInfo.isContainedIn(overrideInfo))
-    return true;
-
-  // Allow overrides that are not as available as the base decl as long as the
-  // override is as available as its context.
-  auto availabilityContext = AvailabilityContext::forDeclSignature(
-      override->getDeclContext()->getSelfNominalTypeDecl());
-
-  return availabilityContext.getPlatformRange().isContainedIn(overrideInfo);
-}
-
-/// Returns true if a diagnostic about an accessor being less available
-/// than the accessor it overrides would be redundant because we will
-/// already emit another diagnostic.
-static bool
-isRedundantAccessorOverrideAvailabilityDiagnostic(ValueDecl *override,
-                                                  ValueDecl *base) {
-
-  auto *overrideFn = dyn_cast<AccessorDecl>(override);
-  auto *baseFn = dyn_cast<AccessorDecl>(base);
-  if (!overrideFn || !baseFn)
-    return false;
-
-  AbstractStorageDecl *overrideASD = overrideFn->getStorage();
-  AbstractStorageDecl *baseASD = baseFn->getStorage();
-  if (overrideASD->getOverriddenDecl() != baseASD)
-    return false;
-
-  // If we have already emitted a diagnostic about an unsafe override
-  // for the property, don't complain about the accessor.
-  if (!isAvailabilitySafeForOverride(overrideASD, baseASD)) {
-    return true;
-  }
-
-  // Returns true if we will already diagnose a bad override
-  // on the property's accessor of the given kind.
-  auto accessorOverrideAlreadyDiagnosed = [&](AccessorKind kind) {
-    FuncDecl *overrideAccessor = overrideASD->getOpaqueAccessor(kind);
-    FuncDecl *baseAccessor = baseASD->getOpaqueAccessor(kind);
-    if (overrideAccessor && baseAccessor &&
-        !isAvailabilitySafeForOverride(overrideAccessor, baseAccessor)) {
-      return true;
-    }
-    return false;
-  };
-
-  // If we have already emitted a diagnostic about an unsafe override
-  // for a getter or a setter, no need to complain about the read or
-  // modify coroutines, which are synthesized to be as available as either
-  // the getter and the setter.
-  switch (overrideFn->getAccessorKind()) {
-  case AccessorKind::Get:
-  case AccessorKind::DistributedGet:
-  case AccessorKind::Set:
-    break;
-
-  case AccessorKind::Read:
-  case AccessorKind::Read2:
-    if (accessorOverrideAlreadyDiagnosed(AccessorKind::Get))
-      return true;
-    break;
-
-  case AccessorKind::Modify:
-  case AccessorKind::Modify2:
-    if (accessorOverrideAlreadyDiagnosed(AccessorKind::Get) ||
-        accessorOverrideAlreadyDiagnosed(AccessorKind::Set)) {
-      return true;
-    }
-    break;
-
-#define OPAQUE_ACCESSOR(ID, KEYWORD)
-#define ACCESSOR(ID, KEYWORD) case AccessorKind::ID:
-#include "swift/AST/AccessorKinds.def"
-    llvm_unreachable("checking override for non-opaque accessor");
-  }
-
-  return false;
-}
-
-/// Diagnose an override for potential availability. Returns true if
-/// a diagnostic was emitted and false otherwise.
-static bool diagnoseOverrideForAvailability(ValueDecl *override,
-                                            ValueDecl *base) {
-  if (isAvailabilitySafeForOverride(override, base))
-    return false;
-
-  // Suppress diagnostics about availability overrides for accessors
-  // if they would be redundant with other diagnostics.
-  if (isRedundantAccessorOverrideAvailabilityDiagnostic(override, base))
-    return false;
-
-  auto &diags = override->getASTContext().Diags;
-  diags.diagnose(override, diag::override_less_available, override);
-  diags.diagnose(base, diag::overridden_here);
-
-  return true;
-}
-
-enum class OverrideUnavailabilityStatus {
   /// The unavailability of the base decl and override decl are compatible.
   Compatible,
   /// The base decl is unavailable but the override decl is not.
   BaseUnavailable,
+  /// The override decl is unavailable but the base decl is not.
+  OverrideUnavailable,
+  /// The override decl is less available than the base decl.
+  OverrideLessAvailable,
   /// Do not diagnose the unavailability of these decls.
   Ignored,
 };
 
-static std::pair<OverrideUnavailabilityStatus,
+static std::pair<OverrideAvailability, std::optional<AvailabilityConstraint>>
-                 std::optional<SemanticAvailableAttr>>
+getOverrideAvailability(ValueDecl *override, ValueDecl *base) {
-checkOverrideUnavailability(ValueDecl *override, ValueDecl *base) {
+  auto &ctx = override->getASTContext();
-  if (auto *overrideParent = override->getDeclContext()->getAsDecl()) {
+
-    // If the parent of the override is unavailable, then the unavailability of
+  // Availability is contravariant so make sure the availability of of an
-    // the override decl is irrelevant.
+  // overridden declaration is fully contained in the availability of the
-    if (AvailabilityContext::forDeclSignature(overrideParent).isUnavailable())
+  // overriding declaration.
-      return {OverrideUnavailabilityStatus::Ignored, std::nullopt};
+  auto baseAvailability = AvailabilityContext::forDeclSignature(base);
+
+  // The override is allowed to be less available than the base decl as long as
+  // it is as available as its containing nominal decl.
+  auto nominalAvailability = AvailabilityContext::forDeclSignature(
+      override->getDeclContext()->getSelfNominalTypeDecl());
+  baseAvailability.constrainWithContext(nominalAvailability, ctx);
+
+  // In order to maintain source compatibility, universally unavailable decls
+  // are allowed to override universally unavailable bases.
+  AvailabilityConstraintFlags flags;
+  flags |= AvailabilityConstraintFlag::
+      AllowUniversallyUnavailableInCompatibleContexts;
+
+  if (auto constraint =
+          getAvailabilityConstraintsForDecl(override, baseAvailability, flags)
+              .getPrimaryConstraint()) {
+    if (constraint->isUnavailable())
+      return {OverrideAvailability::OverrideUnavailable, constraint};
+
+    return {OverrideAvailability::OverrideLessAvailable, constraint};
   }
 
-  if (auto *baseAccessor = dyn_cast<AccessorDecl>(base)) {
+  // Check whether the base is unavailable from the perspective of the override.
-    // Ignore implicit accessors since the diagnostics are likely to duplicate
+  auto overrideAvailability = AvailabilityContext::forDeclSignature(override);
-    // the diagnostics for the explicit accessors that availability was inferred
+  if (auto baseConstraint =
-    // from.
+          getAvailabilityConstraintsForDecl(base, overrideAvailability, flags)
+              .getPrimaryConstraint()) {
+    if (baseConstraint->isUnavailable())
+      return {OverrideAvailability::BaseUnavailable, baseConstraint};
+  }
+
+  return {OverrideAvailability::Compatible, std::nullopt};
+}
+
+static std::pair<OverrideAvailability, std::optional<AvailabilityConstraint>>
+checkOverrideAvailability(ValueDecl *override, ValueDecl *base) {
+  auto &ctx = override->getASTContext();
+  if (ctx.LangOpts.DisableAvailabilityChecking)
+    return {OverrideAvailability::Ignored, std::nullopt};
+
+  auto result = getOverrideAvailability(override, base);
+  switch (result.first) {
+  case OverrideAvailability::Ignored:
+  case OverrideAvailability::Compatible:
+    return result;
+  case OverrideAvailability::BaseUnavailable:
+  case OverrideAvailability::OverrideUnavailable:
+  case OverrideAvailability::OverrideLessAvailable:
+    break;
+  }
+
+  auto *overrideAccessor = dyn_cast<AccessorDecl>(override);
+  auto *baseAccessor = dyn_cast<AccessorDecl>(base);
+  if (baseAccessor && overrideAccessor) {
+    // Skip implicit accessors since they're synthesized with availability that
+    // matches the accessors that they were derived from and therefore
+    // diagnostics for them will be redundant.
     if (baseAccessor->isImplicit())
-      return {OverrideUnavailabilityStatus::Ignored, std::nullopt};
+      return {OverrideAvailability::Ignored, std::nullopt};
 
-    if (auto *overrideAccessor = dyn_cast<AccessorDecl>(override)) {
+    // If we're checking an accessor that's overriding another accessor, ignore
-      // If base and override are accessors, check whether the unavailability of
+    // the result if we get the same result for the underlying storage
-      // their storage matches. Diagnosing accessors with invalid storage
+    // (otherwise we'll emit redundant diagnostics).
-      // produces redundant diagnostics.
+    if (checkOverrideAvailability(overrideAccessor->getStorage(),
-      if (checkOverrideUnavailability(overrideAccessor->getStorage(),
+                                  baseAccessor->getStorage())
-                                      baseAccessor->getStorage())
+            .first != OverrideAvailability::Compatible)
-              .first != OverrideUnavailabilityStatus::Compatible)
+      return {OverrideAvailability::Ignored, std::nullopt};
-        return {OverrideUnavailabilityStatus::Ignored, std::nullopt};
-    }
   }
 
-  auto baseUnavailableAttr = base->getUnavailableAttr();
+  return result;
-  auto overrideUnavailableAttr = override->getUnavailableAttr();
-
-  if (baseUnavailableAttr && !overrideUnavailableAttr)
-    return {OverrideUnavailabilityStatus::BaseUnavailable, baseUnavailableAttr};
-
-  return {OverrideUnavailabilityStatus::Compatible, std::nullopt};
 }
 
 static bool checkSingleOverride(ValueDecl *override, ValueDecl *base) {
@@ -2231,14 +2169,11 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base) {
     return true;
   }
 
-  // FIXME: [availability] Possibly should extend to more availability checking.
+  auto [status, constraint] = checkOverrideAvailability(override, base);
-  auto unavailabilityStatusAndAttr =
+  switch (status) {
-      checkOverrideUnavailability(override, base);
+  case OverrideAvailability::BaseUnavailable: {
-  auto unavailableAttr = unavailabilityStatusAndAttr.second;
+    auto unavailableAttr = constraint->getAttr();
-
+    diagnoseOverrideOfUnavailableDecl(override, base, unavailableAttr);
-  switch (unavailabilityStatusAndAttr.first) {
-  case OverrideUnavailabilityStatus::BaseUnavailable: {
-    diagnoseOverrideOfUnavailableDecl(override, base, unavailableAttr.value());
 
     if (isUnavailableInAllVersions(base)) {
       auto modifier = override->getAttrs().getAttribute<OverrideAttr>();
@@ -2251,13 +2186,32 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base) {
     }
     break;
   }
-  case OverrideUnavailabilityStatus::Compatible:
+  case OverrideAvailability::OverrideUnavailable: {
-  case OverrideUnavailabilityStatus::Ignored:
+    auto unavailableAttr = constraint->getAttr();
+    auto domain = unavailableAttr.getDomain();
+    auto parsedAttr = unavailableAttr.getParsedAttr();
+
+    if (!domain.isCustom()) {
+      // FIXME: [availability] Diagnose as an error in a future Swift version.
+      break;
+    }
+
+    if (parsedAttr->getLocation().isValid())
+      ctx.Diags.diagnose(override, diag::override_unavailable, override)
+          .fixItRemove(parsedAttr->getRangeWithAt());
+    else
+      ctx.Diags.diagnose(override, diag::override_unavailable, override);
+    ctx.Diags.diagnose(base, diag::overridden_here);
     break;
   }
-
+  case OverrideAvailability::OverrideLessAvailable: {
-  if (!ctx.LangOpts.DisableAvailabilityChecking) {
+    ctx.Diags.diagnose(override, diag::override_less_available, override);
-    diagnoseOverrideForAvailability(override, base);
+    ctx.Diags.diagnose(base, diag::overridden_here);
+    break;
+  }
+  case OverrideAvailability::Compatible:
+  case OverrideAvailability::Ignored:
+    break;
   }
 
   if (ctx.LangOpts.hasFeature(Feature::StrictMemorySafety, /*allowMigration=*/true)) {

test/Availability/availability_custom_domains.swift

@@ -367,3 +367,168 @@ struct ConformsToHasAssocTypeRequirementInEnabledDomain3: HasAssocTypeRequiremen
   @available(DisabledDomain)
   struct B { } // expected-error {{protocol 'HasAssocTypeRequirementInEnabledDomain' requirement 'B' cannot be satisfied by struct that is only available in DisabledDomain}}
 }
+
+class Base {
+  func alwaysAvailable() { }
+  // expected-note@-1 * {{overridden declaration is here}}
+
+  @available(EnabledDomain)
+  func availableInEnabledDomain() { }
+  // expected-note@-1 * {{overridden declaration is here}}
+
+  @available(EnabledDomain, unavailable)
+  func unavailableInEnabledDomain() { }
+  // expected-note@-1 * {{overridden declaration is here}}
+  // expected-note@-2 * {{'unavailableInEnabledDomain()' has been explicitly marked unavailable here}}
+}
+
+class Derived1: Base {
+  override func alwaysAvailable() { }
+
+  override func availableInEnabledDomain() { }
+
+  override func unavailableInEnabledDomain() { } // expected-error {{cannot override 'unavailableInEnabledDomain' which has been marked unavailable}}
+  // expected-note@-1 {{remove 'override' modifier to declare a new 'unavailableInEnabledDomain'}}
+}
+
+class Derived2: Base {
+  @available(EnabledDomain)
+  override func alwaysAvailable() { } // expected-error {{overriding 'alwaysAvailable' must be as available as declaration it overrides}}
+
+  @available(EnabledDomain)
+  override func availableInEnabledDomain() { }
+
+  @available(EnabledDomain)
+  override func unavailableInEnabledDomain() { } // expected-error {{overriding 'unavailableInEnabledDomain' must be as available as declaration it overrides}}
+}
+
+@available(EnabledDomain)
+class Derived3: Base {
+  override func alwaysAvailable() { }
+
+  override func availableInEnabledDomain() { }
+
+  override func unavailableInEnabledDomain() { } // expected-error {{cannot override 'unavailableInEnabledDomain' which has been marked unavailable}}
+  // expected-note@-1 {{remove 'override' modifier to declare a new 'unavailableInEnabledDomain'}}
+}
+
+@available(EnabledDomain)
+class Derived4: Base {
+  @available(EnabledDomain)
+  override func alwaysAvailable() { }
+
+  @available(EnabledDomain)
+  override func availableInEnabledDomain() { }
+
+  @available(EnabledDomain)
+  override func unavailableInEnabledDomain() { } // expected-error {{overriding 'unavailableInEnabledDomain' must be as available as declaration it overrides}}
+}
+
+class Derived5: Base {
+  @available(DisabledDomain)
+  override func alwaysAvailable() { } // expected-error {{overriding 'alwaysAvailable' must be as available as declaration it overrides}}
+
+  @available(DisabledDomain)
+  override func availableInEnabledDomain() { } // expected-error {{overriding 'availableInEnabledDomain' must be as available as declaration it overrides}}
+
+  @available(DisabledDomain)
+  override func unavailableInEnabledDomain() { } // expected-error {{overriding 'unavailableInEnabledDomain' must be as available as declaration it overrides}}
+}
+
+@available(DisabledDomain)
+class Derived6: Base {
+  override func alwaysAvailable() { }
+
+  override func availableInEnabledDomain() { }
+
+  override func unavailableInEnabledDomain() { } // expected-error {{cannot override 'unavailableInEnabledDomain' which has been marked unavailable}}
+  // expected-note@-1 {{remove 'override' modifier to declare a new 'unavailableInEnabledDomain'}}
+}
+
+@available(DisabledDomain)
+class Derived7: Base {
+  @available(DisabledDomain)
+  override func alwaysAvailable() { }
+
+  @available(DisabledDomain)
+  override func availableInEnabledDomain() { }
+
+  @available(DisabledDomain)
+  override func unavailableInEnabledDomain() { } // expected-error {{cannot override 'unavailableInEnabledDomain' which has been marked unavailable}}
+  // expected-note@-1 {{remove 'override' modifier to declare a new 'unavailableInEnabledDomain'}}
+}
+
+class Derived8: Base {
+  @available(EnabledDomain, unavailable)
+  override func alwaysAvailable() { } // expected-error {{cannot override 'alwaysAvailable' with a declaration that is marked unavailable}}
+
+  @available(EnabledDomain, unavailable)
+  override func availableInEnabledDomain() { } // expected-error {{cannot override 'availableInEnabledDomain' with a declaration that is marked unavailable}}
+
+  @available(EnabledDomain, unavailable)
+  override func unavailableInEnabledDomain() { }
+}
+
+@available(EnabledDomain, unavailable)
+class Derived9: Base {
+  override func alwaysAvailable() { }
+
+  override func availableInEnabledDomain() { }
+
+  override func unavailableInEnabledDomain() { }
+}
+
+@available(EnabledDomain, unavailable)
+class Derived10: Base {
+  @available(EnabledDomain, unavailable)
+  override func alwaysAvailable() { }
+
+  @available(EnabledDomain, unavailable)
+  override func availableInEnabledDomain() { }
+
+  @available(EnabledDomain, unavailable)
+  override func unavailableInEnabledDomain() { }
+}
+
+@available(EnabledDomain, unavailable)
+class Derived11: Base {
+  @available(EnabledDomain)
+  override func alwaysAvailable() { } // expected-error {{overriding 'alwaysAvailable' must be as available as declaration it overrides}}
+
+  @available(EnabledDomain)
+  override func availableInEnabledDomain() { } // expected-error {{overriding 'availableInEnabledDomain' must be as available as declaration it overrides}}
+
+  @available(EnabledDomain)
+  override func unavailableInEnabledDomain() { } // expected-error {{overriding 'unavailableInEnabledDomain' must be as available as declaration it overrides}}
+}
+
+@available(EnabledDomain)
+class BaseAvailableInEnabledDomain { }
+
+class DerivedMoreAvailable: BaseAvailableInEnabledDomain { // expected-error {{'BaseAvailableInEnabledDomain' is only available in EnabledDomain}}
+  // expected-note@-1 {{add '@available' attribute to enclosing class}}
+}
+
+@available(EnabledDomain)
+class DerivedAsAvailable: BaseAvailableInEnabledDomain { }
+
+@available(EnabledDomain)
+@available(DisabledDomain)
+class DerivedAsAvailable2: BaseAvailableInEnabledDomain { }
+
+@available(DisabledDomain)
+class DerivedLessAvailable: BaseAvailableInEnabledDomain { // expected-error {{'BaseAvailableInEnabledDomain' is only available in EnabledDomain}}
+  // expected-note@-1 {{add '@available' attribute to enclosing class}}
+}
+
+@available(EnabledDomain, unavailable)
+class DerivedUnavailable: BaseAvailableInEnabledDomain { }
+
+// FIXME: This shouldn't be accepted
+@available(DisabledDomain, unavailable)
+class DerivedUnavailable2: BaseAvailableInEnabledDomain { }
+
+@available(EnabledDomain)
+@available(DisabledDomain, unavailable)
+class DerivedUnavailable3: BaseAvailableInEnabledDomain { }
+