AST: Use consolidated availability constraint query for diagnostics.

Switch to calling `swift::getAvailabilityConstraintsForDecl()` to get the
unsatisfied availability constraints that should be diagnosed.

This was intended to be NFC, but it turns out it fixed a bug in the recently
introduced objc_implementation_direct_to_storage.swift test. In the test,
the stored properties are as unavailable as the context that is accessing them
so the accesses should not be diagnosed. However, this test demonstrates a
bigger issue with `@objc @implementation`, which is that it allows the
implementations of Obj-C interfaces to be less available than the interface,
which effectively provides an availability checking loophole that can be used
to invoke unavailable code.

Author: tshortli

include/swift/AST/AvailabilityConstraint.h

@@ -33,6 +33,10 @@ class Decl;
 /// certain context.
 class AvailabilityConstraint {
 public:
+  /// The reason that the availability constraint is unsatisfied.
+  ///
+  /// NOTE: The order of this enum matters. Reasons are defined in descending
+  /// priority order.
   enum class Reason {
     /// The declaration is referenced in a context in which it is generally
     /// unavailable. For example, a reference to a declaration that is
@@ -133,38 +137,34 @@ class AvailabilityConstraint {
   /// Returns the required range for `IntroducedInNewerVersion` requirements, or
   /// `std::nullopt` otherwise.
   std::optional<AvailabilityRange>
-  getRequiredNewerAvailabilityRange(ASTContext &ctx) const;
+  getRequiredNewerAvailabilityRange(const ASTContext &ctx) const;
 
   /// Some availability constraints are active for type-checking but cannot
   /// be translated directly into an `if #available(...)` runtime query.
-  bool isActiveForRuntimeQueries(ASTContext &ctx) const;
+  bool isActiveForRuntimeQueries(const ASTContext &ctx) const;
 };
 
 /// Represents a set of availability constraints that restrict use of a
-/// declaration in a particular context.
+/// declaration in a particular context. There can only be one active constraint
+/// for a given `AvailabilityDomain`, but there may be multiple active
+/// constraints from separate domains.
 class DeclAvailabilityConstraints {
   using Storage = llvm::SmallVector<AvailabilityConstraint, 4>;
   Storage constraints;
 
 public:
   DeclAvailabilityConstraints() {}
+  DeclAvailabilityConstraints(const Storage &&constraints)
+      : constraints(constraints) {}
 
-  void addConstraint(const AvailabilityConstraint &constraint) {
-    constraints.emplace_back(constraint);
-  }
+  /// Returns the strongest availability constraint or `std::nullopt` if empty.
+  std::optional<AvailabilityConstraint> getPrimaryConstraint() const;
 
   using const_iterator = Storage::const_iterator;
   const_iterator begin() const { return constraints.begin(); }
   const_iterator end() const { return constraints.end(); }
 };
 
-/// Returns the `AvailabilityConstraint` that describes how \p attr restricts
-/// use of \p decl in \p context or `std::nullopt` if there is no restriction.
-std::optional<AvailabilityConstraint>
-getAvailabilityConstraintForAttr(const Decl *decl,
-                                 const SemanticAvailableAttr &attr,
-                                 const AvailabilityContext &context);
-
 /// Returns the set of availability constraints that restrict use of \p decl
 /// when it is referenced from the given context. In other words, it is the
 /// collection of of `@available` attributes with unsatisfied conditions.

lib/AST/Availability.cpp

@@ -573,49 +573,13 @@ bool Decl::isUnavailableInCurrentSwiftVersion() const {
   return false;
 }
 
-std::optional<SemanticAvailableAttr> getDeclUnavailableAttr(const Decl *D) {
-  auto &ctx = D->getASTContext();
-  std::optional<SemanticAvailableAttr> result;
-  auto bestActive = D->getActiveAvailableAttrForCurrentPlatform();
-
-  for (auto attr : D->getSemanticAvailableAttrs(/*includingInactive=*/false)) {
-    // If this is a platform-specific attribute and it isn't the most
-    // specific attribute for the current platform, we're done.
-    if (attr.isPlatformSpecific() && (!bestActive || attr != bestActive))
-      continue;
-
-    // Unconditional unavailable.
-    if (attr.isUnconditionallyUnavailable())
-      return attr;
-
-    switch (attr.getVersionAvailability(ctx)) {
-    case AvailableVersionComparison::Available:
-    case AvailableVersionComparison::PotentiallyUnavailable:
-      break;
-
-    case AvailableVersionComparison::Obsoleted:
-    case AvailableVersionComparison::Unavailable:
-      result.emplace(attr);
-      break;
-    }
-  }
-  return result;
-}
-
 std::optional<SemanticAvailableAttr> Decl::getUnavailableAttr() const {
-  if (auto attr = getDeclUnavailableAttr(this))
-    return attr;
-
-  // If D is an extension member, check if the extension is unavailable.
-  //
-  // Skip decls imported from Clang, they could be associated to the wrong
-  // extension and inherit undesired unavailability. The ClangImporter
-  // associates Objective-C protocol members to the first category where the
-  // protocol is directly or indirectly adopted, no matter its availability
-  // and the availability of other categories. rdar://problem/53956555
-  if (!getClangNode())
-    if (auto ext = dyn_cast<ExtensionDecl>(getDeclContext()))
-      return ext->getUnavailableAttr();
+  auto context = AvailabilityContext::forDeploymentTarget(getASTContext());
+  if (auto constraint = getAvailabilityConstraintsForDecl(this, context)
+                            .getPrimaryConstraint()) {
+    if (constraint->isUnavailable())
+      return constraint->getAttr();
+  }
 
   return std::nullopt;
 }

lib/AST/AvailabilityConstraint.cpp

@@ -24,7 +24,7 @@ PlatformKind AvailabilityConstraint::getPlatform() const {
 
 std::optional<AvailabilityRange>
 AvailabilityConstraint::getRequiredNewerAvailabilityRange(
-    ASTContext &ctx) const {
+    const ASTContext &ctx) const {
   switch (getReason()) {
   case Reason::UnconditionallyUnavailable:
   case Reason::Obsoleted:
@@ -35,7 +35,8 @@ AvailabilityConstraint::getRequiredNewerAvailabilityRange(
   }
 }
 
-bool AvailabilityConstraint::isActiveForRuntimeQueries(ASTContext &ctx) const {
+bool AvailabilityConstraint::isActiveForRuntimeQueries(
+    const ASTContext &ctx) const {
   if (getAttr().getPlatform() == PlatformKind::none)
     return true;
 
@@ -44,6 +45,74 @@ bool AvailabilityConstraint::isActiveForRuntimeQueries(ASTContext &ctx) const {
                                  /*forRuntimeQuery=*/true);
 }
 
+static bool constraintIsStronger(const AvailabilityConstraint &lhs,
+                                 const AvailabilityConstraint &rhs) {
+  DEBUG_ASSERT(lhs.getDomain() == rhs.getDomain());
+
+  // If the constraints have matching domains but different reasons, the
+  // constraint with the lowest reason is "strongest".
+  if (lhs.getReason() != rhs.getReason())
+    return lhs.getReason() < rhs.getReason();
+
+  switch (lhs.getReason()) {
+  case AvailabilityConstraint::Reason::UnconditionallyUnavailable:
+    // Just keep the first.
+    return false;
+
+  case AvailabilityConstraint::Reason::Obsoleted:
+    // Pick the earliest obsoleted version.
+    return *lhs.getAttr().getObsoleted() < *rhs.getAttr().getObsoleted();
+
+  case AvailabilityConstraint::Reason::IntroducedInLaterVersion:
+  case AvailabilityConstraint::Reason::IntroducedInLaterDynamicVersion:
+    // Pick the latest introduced version.
+    return *lhs.getAttr().getIntroduced() > *rhs.getAttr().getIntroduced();
+  }
+}
+
+void addConstraint(llvm::SmallVector<AvailabilityConstraint, 4> &constraints,
+                   const AvailabilityConstraint &constraint,
+                   const ASTContext &ctx) {
+
+  auto iter = llvm::find_if(
+      constraints, [&constraint](AvailabilityConstraint &existing) {
+        return constraint.getDomain() == existing.getDomain();
+      });
+
+  // There's no existing constraint for the same domain so just add it.
+  if (iter == constraints.end()) {
+    constraints.emplace_back(constraint);
+    return;
+  }
+
+  if (constraintIsStronger(constraint, *iter)) {
+    constraints.erase(iter);
+    constraints.emplace_back(constraint);
+  }
+}
+
+std::optional<AvailabilityConstraint>
+DeclAvailabilityConstraints::getPrimaryConstraint() const {
+  std::optional<AvailabilityConstraint> result;
+
+  auto isStrongerConstraint = [](const AvailabilityConstraint &lhs,
+                                 const AvailabilityConstraint &rhs) {
+    // Pick the strongest constraint. Constraint kinds are defined in descending
+    // order of strength.
+    if (lhs.getKind() != rhs.getKind())
+      return lhs.getKind() < rhs.getKind();
+
+    return false;
+  };
+
+  for (auto const &constraint : constraints) {
+    if (!result || isStrongerConstraint(constraint, *result))
+      result.emplace(constraint);
+  }
+
+  return result;
+}
+
 static bool
 isInsideCompatibleUnavailableDeclaration(const Decl *decl,
                                          const SemanticAvailableAttr &attr,
@@ -65,13 +134,12 @@ isInsideCompatibleUnavailableDeclaration(const Decl *decl,
   return context.containsUnavailableDomain(domain);
 }
 
-std::optional<AvailabilityConstraint>
-swift::getAvailabilityConstraintForAttr(const Decl *decl,
-                                        const SemanticAvailableAttr &attr,
-                                        const AvailabilityContext &context) {
-  if (isInsideCompatibleUnavailableDeclaration(decl, attr, context))
-    return std::nullopt;
-
+/// Returns the `AvailabilityConstraint` that describes how \p attr restricts
+/// use of \p decl in \p context or `std::nullopt` if there is no restriction.
+static std::optional<AvailabilityConstraint>
+getAvailabilityConstraintForAttr(const Decl *decl,
+                                 const SemanticAvailableAttr &attr,
+                                 const AvailabilityContext &context) {
   if (attr.isUnconditionallyUnavailable())
     return AvailabilityConstraint::unconditionallyUnavailable(attr);
 
@@ -128,10 +196,10 @@ activePlatformDomainForDecl(const Decl *decl) {
   return activeDomain;
 }
 
-static void
-getAvailabilityConstraintsForDecl(DeclAvailabilityConstraints &constraints,
-                                  const Decl *decl,
-                                  const AvailabilityContext &context) {
+static void getAvailabilityConstraintsForDecl(
+    llvm::SmallVector<AvailabilityConstraint, 4> &constraints, const Decl *decl,
+    const AvailabilityContext &context) {
+  auto &ctx = decl->getASTContext();
   auto activePlatformDomain = activePlatformDomainForDecl(decl);
 
   for (auto attr :
@@ -141,20 +209,27 @@ getAvailabilityConstraintsForDecl(DeclAvailabilityConstraints &constraints,
         !activePlatformDomain->contains(domain))
       continue;
 
-    if (auto constraint =
-            swift::getAvailabilityConstraintForAttr(decl, attr, context))
-      constraints.addConstraint(*constraint);
+    if (auto constraint = getAvailabilityConstraintForAttr(decl, attr, context))
+      addConstraint(constraints, *constraint, ctx);
   }
+
+  // After resolving constraints, remove any constraints that indicate the
+  // declaration is unconditionally unavailable in a domain for which
+  // the context is already unavailable.
+  llvm::erase_if(constraints, [&](const AvailabilityConstraint &constraint) {
+    return isInsideCompatibleUnavailableDeclaration(decl, constraint.getAttr(),
+                                                    context);
+  });
 }
 
 DeclAvailabilityConstraints
 swift::getAvailabilityConstraintsForDecl(const Decl *decl,
                                          const AvailabilityContext &context) {
-  DeclAvailabilityConstraints constraints;
+  llvm::SmallVector<AvailabilityConstraint, 4> constraints;
 
   // Generic parameters are always available.
   if (isa<GenericTypeParamDecl>(decl))
-    return constraints;
+    return DeclAvailabilityConstraints();
 
   decl = abstractSyntaxDeclForAvailableAttribute(decl);
 

lib/Sema/TypeCheckAvailability.cpp

@@ -3088,44 +3088,9 @@ bool diagnoseExplicitUnavailability(SourceLoc loc,
 std::optional<AvailabilityConstraint>
 swift::getUnsatisfiedAvailabilityConstraint(
     const Decl *decl, AvailabilityContext availabilityContext) {
-  auto &ctx = decl->getASTContext();
-
-  // Generic parameters are always available.
-  if (isa<GenericTypeParamDecl>(decl))
-    return std::nullopt;
-
-  if (auto attr = decl->getUnavailableAttr()) {
-    if (isInsideCompatibleUnavailableDeclaration(decl, availabilityContext,
-                                                 *attr))
-      return std::nullopt;
-
-    switch (attr->getVersionAvailability(ctx)) {
-    case AvailableVersionComparison::Available:
-    case AvailableVersionComparison::PotentiallyUnavailable:
-      llvm_unreachable("Decl should be unavailable");
-
-    case AvailableVersionComparison::Unavailable:
-      if ((attr->isSwiftLanguageModeSpecific() ||
-           attr->isPackageDescriptionVersionSpecific()) &&
-          attr->getIntroduced().has_value())
-        return AvailabilityConstraint::introducedInLaterVersion(*attr);
-
-      return AvailabilityConstraint::unconditionallyUnavailable(*attr);
-
-    case AvailableVersionComparison::Obsoleted:
-      return AvailabilityConstraint::obsoleted(*attr);
-    }
-  }
-
-  // Check whether the declaration is available in a newer platform version.
-  if (auto rangeAttr = decl->getAvailableAttrForPlatformIntroduction()) {
-    auto range = rangeAttr->getIntroducedRange(ctx);
-    if (!availabilityContext.getPlatformRange().isContainedIn(range))
-      return AvailabilityConstraint::introducedInLaterDynamicVersion(
-          *rangeAttr);
-  }
-
-  return std::nullopt;
+  auto constraints =
+      swift::getAvailabilityConstraintsForDecl(decl, availabilityContext);
+  return constraints.getPrimaryConstraint();
 }
 
 std::optional<AvailabilityConstraint>

test/Sema/availability_scopes.swift

@@ -358,7 +358,7 @@ extension SomeEnum {
 // CHECK-NEXT: {{^}}  (decl_implicit version=50 decl=extension.SomeEnum
 // CHECK-NEXT: {{^}}    (decl version=50 unavailable=macOS decl=extension.SomeEnum
 // CHECK-NEXT: {{^}}      (decl_implicit version=50 unavailable=macOS decl=availableMacOS_52
-// CHECK-NEXT: {{^}}        (decl version=52 unavailable=macOS decl=availableMacOS_52
+// CHECK-NEXT: {{^}}        (decl version=50 unavailable=macOS decl=availableMacOS_52
 // CHECK-NEXT: {{^}}      (decl version=50 unavailable=* decl=neverAvailable()
 
 @available(macOS, unavailable)

test/attr/attr_availability_transitive_osx.swift

@@ -533,6 +533,9 @@ func available_func_call_extension_methods(_ e: ExtendMe) { // expected-note {{a
   // expected-note@-1 {{add 'if #available' version check}}
 }
 
+@available(OSX, obsoleted: 10.9)
+struct OSXObsoleted {} // expected-note 2 {{'OSXObsoleted' was obsoleted in macOS 10.9}}
+
 @available(OSX, unavailable)
 @available(OSX, introduced: 99)
 struct OSXUnavailableAndIntroducedInFuture {}
@@ -547,32 +550,33 @@ struct OSXIntroducedInFutureAndUnavailable {}
 @available(OSX, unavailable)
 func osx_unavailable_func(
   _ s1: OSXFutureAvailable,
-  _ s2: OSXUnavailableAndIntroducedInFuture,
-  _ s3: OSXUnavailableAndIntroducedInFutureSameAttribute,
-  _ s4: OSXIntroducedInFutureAndUnavailable,
+  _ s2: OSXObsoleted,
+  _ s3: OSXUnavailableAndIntroducedInFuture,
+  _ s4: OSXUnavailableAndIntroducedInFutureSameAttribute,
+  _ s5: OSXIntroducedInFutureAndUnavailable,
 ) -> (
   OSXFutureAvailable,
+  OSXObsoleted,
   OSXUnavailableAndIntroducedInFuture,
   OSXUnavailableAndIntroducedInFutureSameAttribute,
   OSXIntroducedInFutureAndUnavailable
 ) {
+  // FIXME: [availability] Stop diagnosing potential unavailability or obsoletion in an unavailable context.
   _ = OSXFutureAvailable() // expected-error {{'OSXFutureAvailable' is only available in macOS 99 or newer}}
   // expected-note@-1 {{add 'if #available' version check}}
-  // FIXME: [availability] The following diagnostic is incorrect
-  _ = OSXUnavailableAndIntroducedInFuture() // expected-error {{'OSXUnavailableAndIntroducedInFuture' is only available in macOS 99 or newer}}
-  // expected-note@-1 {{add 'if #available' version check}}
+  _ = OSXObsoleted() // expected-error {{'OSXObsoleted' is unavailable in macOS}}
+  _ = OSXUnavailableAndIntroducedInFuture()
   _ = OSXUnavailableAndIntroducedInFutureSameAttribute()
   _ = OSXIntroducedInFutureAndUnavailable()
 
   func takesType<T>(_ t: T.Type) {}
   takesType(OSXFutureAvailable.self) // expected-error {{'OSXFutureAvailable' is only available in macOS 99 or newer}}
   // expected-note@-1 {{add 'if #available' version check}}
-  // FIXME: [availability] The following diagnostic is incorrect
-  takesType(OSXUnavailableAndIntroducedInFuture.self) // expected-error {{'OSXUnavailableAndIntroducedInFuture' is only available in macOS 99 or newer}}
-  // expected-note@-1 {{add 'if #available' version check}}
+  takesType(OSXObsoleted.self) // expected-error {{'OSXObsoleted' is unavailable in macOS}}
+  takesType(OSXUnavailableAndIntroducedInFuture.self)
   takesType(OSXUnavailableAndIntroducedInFutureSameAttribute.self)
   takesType(OSXIntroducedInFutureAndUnavailable.self)
 
-  return (s1, s2, s3, s4)
+  return (s1, s2, s3, s4, s5)
 }