Merge pull request #83354 from tshortli/check-custom-domain-availability-for-overrides

Sema: Check custom domain availability for overrides

Author: tshortli

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,
-      "cannot override %0 which has been marked unavailable%select{|: %1}1",
-      (DeclBaseName, StringRef))
+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,8 +1630,8 @@ void swift::diagnoseOverrideOfUnavailableDecl(ValueDecl *override,
   auto &diags = ctx.Diags;
   if (attr.getRename().empty()) {
     EncodedDiagnosticMessage EncodedMessage(attr.getMessage());
-    diags.diagnose(override, diag::override_unavailable,
-                   override->getBaseName(), EncodedMessage.Message);
+    diags.diagnose(override, diag::cannot_override_unavailable,
+                   override, EncodedMessage.Message);
 
     diags.diagnose(base, diag::availability_marked_unavailable, base);
     return;

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
-/// 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;
+enum class OverrideAvailability {
+  /// 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,
+};
 
-  // 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());
+static std::pair<OverrideAvailability, std::optional<AvailabilityConstraint>>
+getOverrideAvailability(ValueDecl *override, ValueDecl *base) {
+  auto &ctx = override->getASTContext();
 
-  return availabilityContext.getPlatformRange().isContainedIn(overrideInfo);
-}
+  // Availability is contravariant so make sure the availability of of an
+  // overridden declaration is fully contained in the availability of the
+  // overriding declaration.
+  auto baseAvailability = AvailabilityContext::forDeclSignature(base);
 
-/// 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) {
+  // 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);
 
-  auto *overrideFn = dyn_cast<AccessorDecl>(override);
-  auto *baseFn = dyn_cast<AccessorDecl>(base);
-  if (!overrideFn || !baseFn)
-    return false;
+  // In order to maintain source compatibility, universally unavailable decls
+  // are allowed to override universally unavailable bases.
+  AvailabilityConstraintFlags flags;
+  flags |= AvailabilityConstraintFlag::
+      AllowUniversallyUnavailableInCompatibleContexts;
 
-  AbstractStorageDecl *overrideASD = overrideFn->getStorage();
-  AbstractStorageDecl *baseASD = baseFn->getStorage();
-  if (overrideASD->getOverriddenDecl() != baseASD)
-    return false;
+  if (auto constraint =
+          getAvailabilityConstraintsForDecl(override, baseAvailability, flags)
+              .getPrimaryConstraint()) {
+    if (constraint->isUnavailable())
+      return {OverrideAvailability::OverrideUnavailable, constraint};
 
-  // 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;
+    return {OverrideAvailability::OverrideLessAvailable, constraint};
   }
 
-  // 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");
+  // Check whether the base is unavailable from the perspective of the override.
+  auto overrideAvailability = AvailabilityContext::forDeclSignature(override);
+  if (auto baseConstraint =
+          getAvailabilityConstraintsForDecl(base, overrideAvailability, flags)
+              .getPrimaryConstraint()) {
+    if (baseConstraint->isUnavailable())
+      return {OverrideAvailability::BaseUnavailable, baseConstraint};
   }
 
-  return false;
+  return {OverrideAvailability::Compatible, std::nullopt};
 }
 
-/// 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,
-  /// Do not diagnose the unavailability of these decls.
-  Ignored,
-};
-
-static std::pair<OverrideUnavailabilityStatus,
-                 std::optional<SemanticAvailableAttr>>
-checkOverrideUnavailability(ValueDecl *override, ValueDecl *base) {
-  if (auto *overrideParent = override->getDeclContext()->getAsDecl()) {
-    // If the parent of the override is unavailable, then the unavailability of
-    // the override decl is irrelevant.
-    if (AvailabilityContext::forDeclSignature(overrideParent).isUnavailable())
-      return {OverrideUnavailabilityStatus::Ignored, 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;
   }
 
-  if (auto *baseAccessor = dyn_cast<AccessorDecl>(base)) {
-    // Ignore implicit accessors since the diagnostics are likely to duplicate
-    // the diagnostics for the explicit accessors that availability was inferred
-    // from.
+  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 base and override are accessors, check whether the unavailability of
-      // their storage matches. Diagnosing accessors with invalid storage
-      // produces redundant diagnostics.
-      if (checkOverrideUnavailability(overrideAccessor->getStorage(),
-                                      baseAccessor->getStorage())
-              .first != OverrideUnavailabilityStatus::Compatible)
-        return {OverrideUnavailabilityStatus::Ignored, std::nullopt};
-    }
+    // If we're checking an accessor that's overriding another accessor, ignore
+    // the result if we get the same result for the underlying storage
+    // (otherwise we'll emit redundant diagnostics).
+    if (checkOverrideAvailability(overrideAccessor->getStorage(),
+                                  baseAccessor->getStorage())
+            .first != OverrideAvailability::Compatible)
+      return {OverrideAvailability::Ignored, std::nullopt};
   }
 
-  auto baseUnavailableAttr = base->getUnavailableAttr();
-  auto overrideUnavailableAttr = override->getUnavailableAttr();
-
-  if (baseUnavailableAttr && !overrideUnavailableAttr)
-    return {OverrideUnavailabilityStatus::BaseUnavailable, baseUnavailableAttr};
-
-  return {OverrideUnavailabilityStatus::Compatible, std::nullopt};
+  return result;
 }
 
 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 unavailabilityStatusAndAttr =
-      checkOverrideUnavailability(override, base);
-  auto unavailableAttr = unavailabilityStatusAndAttr.second;
-
-  switch (unavailabilityStatusAndAttr.first) {
-  case OverrideUnavailabilityStatus::BaseUnavailable: {
-    diagnoseOverrideOfUnavailableDecl(override, base, unavailableAttr.value());
+  auto [status, constraint] = checkOverrideAvailability(override, base);
+  switch (status) {
+  case OverrideAvailability::BaseUnavailable: {
+    auto unavailableAttr = constraint->getAttr();
+    diagnoseOverrideOfUnavailableDecl(override, base, unavailableAttr);
 
     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 OverrideUnavailabilityStatus::Ignored:
+  case OverrideAvailability::OverrideUnavailable: {
+    auto unavailableAttr = constraint->getAttr();
+    auto domain = unavailableAttr.getDomain();
+    auto parsedAttr = unavailableAttr.getParsedAttr();
+
+    if (domain.isPlatform() || domain.isUniversal()) {
+      // 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: {
+    ctx.Diags.diagnose(override, diag::override_less_available, override);
+    ctx.Diags.diagnose(base, diag::overridden_here);
     break;
   }
-
-  if (!ctx.LangOpts.DisableAvailabilityChecking) {
-    diagnoseOverrideForAvailability(override, base);
+  case OverrideAvailability::Compatible:
+  case OverrideAvailability::Ignored:
+    break;
   }
 
   if (ctx.LangOpts.hasFeature(Feature::StrictMemorySafety, /*allowMigration=*/true)) {