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

@@ -133,11 +133,11 @@ 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
@@ -149,9 +149,14 @@ class DeclAvailabilityConstraints {
 public:
   DeclAvailabilityConstraints() {}
 
-  void addConstraint(const AvailabilityConstraint &constraint) {
-    constraints.emplace_back(constraint);
-  }
+  /// Inserts a constraint into the collection. If the constraint has the same
+  /// kind and domain as an existing constraint, then the weaker constraint is
+  /// discarded.
+  void addConstraint(const AvailabilityConstraint &constraint,
+                     const ASTContext &ctx);
+
+  /// 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(); }

lib/AST/Availability.cpp

@@ -575,11 +575,10 @@ bool Decl::isUnavailableInCurrentSwiftVersion() const {
 
 std::optional<SemanticAvailableAttr> Decl::getUnavailableAttr() const {
   auto context = AvailabilityContext::forDeploymentTarget(getASTContext());
-  auto constraints = swift::getAvailabilityConstraintsForDecl(this, context);
-
-  for (auto const &constraint : constraints) {
-    if (constraint.isUnavailable())
-      return constraint.getAttr();
+  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,59 @@ bool AvailabilityConstraint::isActiveForRuntimeQueries(ASTContext &ctx) const {
                                  /*forRuntimeQuery=*/true);
 }
 
+void DeclAvailabilityConstraints::addConstraint(
+    const AvailabilityConstraint &constraint, const ASTContext &ctx) {
+
+  auto existing = llvm::find_if(
+      constraints, [&constraint](AvailabilityConstraint &existing) {
+        return constraint.getKind() == existing.getKind() &&
+               constraint.getDomain() == existing.getDomain();
+      });
+
+  if (existing == constraints.end()) {
+    constraints.emplace_back(constraint);
+    return;
+  }
+
+  // Since the constraints have matching kinds and domains, choose the stronger
+  // constraint to keep.
+  switch (constraint.getKind()) {
+  case AvailabilityConstraint::Kind::Unavailable:
+    // All unavailable constraints are considered equal, just keep the first
+    // we encounter.
+    break;
+  case AvailabilityConstraint::Kind::PotentiallyAvailable:
+    // Replace the existing constraint if the required version is higher.
+    auto existingRange = existing->getRequiredNewerAvailabilityRange(ctx);
+    auto newRange = constraint.getRequiredNewerAvailabilityRange(ctx);
+    if (existingRange && newRange && newRange->isContainedIn(*existingRange))
+      *existing = constraint;
+    break;
+  }
+}
+
+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,
@@ -132,6 +186,7 @@ static void
 getAvailabilityConstraintsForDecl(DeclAvailabilityConstraints &constraints,
                                   const Decl *decl,
                                   const AvailabilityContext &context) {
+  auto &ctx = decl->getASTContext();
   auto activePlatformDomain = activePlatformDomainForDecl(decl);
 
   for (auto attr :
@@ -143,7 +198,7 @@ getAvailabilityConstraintsForDecl(DeclAvailabilityConstraints &constraints,
 
     if (auto constraint =
             swift::getAvailabilityConstraintForAttr(decl, attr, context))
-      constraints.addConstraint(*constraint);
+      constraints.addConstraint(*constraint, ctx);
   }
 }
 

lib/Sema/TypeCheckAvailability.cpp

@@ -3087,44 +3087,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/decl/ext/objc_implementation_direct_to_storage.swift

@@ -4,22 +4,20 @@
 
 import objc_implementation_internal
 
+// FIXME: [availability] An implementation that is less available than the interface it implements should be diagnosed
 @available(*, unavailable)
 @objc @implementation extension ObjCPropertyTest {
-  // FIXME: Shouldn't this be on the `@available` above?
-  // expected-note@+1 {{'prop1' has been explicitly marked unavailable here}}
   let prop1: Int32
 
-  // expected-note@+1 2 {{'prop2' has been explicitly marked unavailable here}}
   var prop2: Int32 {
     didSet {
-      _ = prop2 // expected-error {{'prop2' is unavailable}}
+      _ = prop2
     }
   }
 
   override init() {
-    self.prop1 = 1 // expected-error {{'prop1' is unavailable}}
-    self.prop2 = 2 // expected-error {{'prop2' is unavailable}}
+    self.prop1 = 1
+    self.prop2 = 2
     super.init()
   }
 
@@ -28,3 +26,8 @@ import objc_implementation_internal
     _ = self.prop2
   }
 }
+
+func takesObjCPropertyTest(_ o: ObjCPropertyTest) {
+  _ = o.prop1
+  _ = o.prop2
+}