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,78 @@ 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) {
+    // Constraint reasons are defined in descending order of strength.
+    if (lhs.getReason() != rhs.getReason())
+      return lhs.getReason() < rhs.getReason();
+
+    // Pick the constraint from the broader domain.
+    if (lhs.getDomain() != rhs.getDomain())
+      return rhs.getDomain().contains(lhs.getDomain());
+    
+    return false;
+  };
+
+  // Pick the strongest constraint.
+  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 +138,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 +200,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 +213,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

@@ -3000,11 +3000,10 @@ bool shouldHideDomainNameForConstraintDiagnostic(
   case AvailabilityDomain::Kind::Universal:
   case AvailabilityDomain::Kind::Embedded:
   case AvailabilityDomain::Kind::Custom:
+  case AvailabilityDomain::Kind::PackageDescription:
     return true;
   case AvailabilityDomain::Kind::Platform:
     return false;
-
-  case AvailabilityDomain::Kind::PackageDescription:
   case AvailabilityDomain::Kind::SwiftLanguage:
     switch (constraint.getReason()) {
     case AvailabilityConstraint::Reason::UnconditionallyUnavailable:
@@ -3081,44 +3080,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_swiftpm_v4.swift

@@ -37,6 +37,9 @@ func fourPointOh() {}
 @available(_PackageDescription 4)
 class ShortFour {}
 
+@available(_PackageDescription 99)
+func ninetyNine() {} // expected-note {{'ninetyNine()' was introduced in PackageDescription 99}}
+
 shortThree()
 threePointOh()
 threePointOhOnly() // expected-error {{is unavailable}}
@@ -51,6 +54,7 @@ shortFourPointOh()
 four()
 fourPointOh()
 let aa : ShortFour
+ninetyNine() // expected-error {{'ninetyNine()' is unavailable}}
 
 @available(_PackageDescription, introduced: 4.0)
 @available(*, deprecated, message: "test deprecated")
@@ -63,3 +67,15 @@ func shouldBeAlone() {}
 
 @available(_PackageDescription 4.0, swift 2.0, *) // expected-error {{'_PackageDescription' version-availability must be specified alone}} // expected-error {{'swift' version-availability must be specified alone}}
 func shouldBeAlone2() {} 
+
+@available(*, unavailable, renamed: "shortFour")
+@available(_PackageDescription 3)
+func unconditionallyRenamed() {} // expected-note {{'unconditionallyRenamed()' has been explicitly marked unavailable here}}
+
+unconditionallyRenamed() // expected-error {{'unconditionallyRenamed()' has been renamed to 'shortFour'}}
+
+@available(*, unavailable, renamed: "shortFour")
+@available(_PackageDescription 5)
+func unconditionallyRenamedAndIntroducedLater() {} // expected-note {{'unconditionallyRenamedAndIntroducedLater()' has been explicitly marked unavailable here}}
+
+unconditionallyRenamedAndIntroducedLater() // expected-error {{'unconditionallyRenamedAndIntroducedLater()' has been renamed to 'shortFour'}}