Merge pull request #84454 from tshortli/deprecated-custom-availability-domain-fix-its

Author: tshortli

include/swift/AST/AvailabilityDomain.h

@@ -289,6 +289,12 @@ class AvailabilityDomain final {
     return getRemappedDomain(ctx, unused);
   }
 
+  /// Returns true for a domain that is permanently always available, and
+  /// therefore availability constraints in the domain are effectively the same
+  /// as constraints in the `*` domain. This is used to diagnose unnecessary
+  /// `@available` attributes and `if #available` statements.
+  bool isPermanentlyAlwaysEnabled() const;
+
   bool operator==(const AvailabilityDomain &other) const {
     return storage.getOpaqueValue() == other.storage.getOpaqueValue();
   }
@@ -330,7 +336,7 @@ struct StableAvailabilityDomainComparator {
 /// Represents an availability domain that has been defined in a module.
 class CustomAvailabilityDomain : public llvm::FoldingSetNode {
 public:
-  enum class Kind {
+  enum class Kind : uint8_t {
     /// A domain that is known to be enabled at compile time.
     Enabled,
     /// A domain that is known to be enabled at compile time and is also assumed
@@ -344,10 +350,20 @@ class CustomAvailabilityDomain : public llvm::FoldingSetNode {
 
 private:
   Identifier name;
-  Kind kind;
   ModuleDecl *mod;
   ValueDecl *decl;
   FuncDecl *predicateFunc;
+  Kind kind;
+
+  struct {
+    /// Whether the "isPermanentlyEnabled" bit has been computed yet.
+    unsigned isPermanentlyEnabledComputed : 1;
+    /// Whether the domain is permanently enabled, which makes constraints in
+    /// the domain equivalent to those in the `*` domain.
+    unsigned isPermanentlyEnabled : 1;
+  } flags = {};
+
+  friend class IsCustomAvailabilityDomainPermanentlyEnabled;
 
   CustomAvailabilityDomain(Identifier name, Kind kind, ModuleDecl *mod,
                            ValueDecl *decl, FuncDecl *predicateFunc);
@@ -375,6 +391,11 @@ class CustomAvailabilityDomain : public llvm::FoldingSetNode {
   void Profile(llvm::FoldingSetNodeID &ID) const { Profile(ID, name, mod); }
 };
 
+inline void simple_display(llvm::raw_ostream &os,
+                           const CustomAvailabilityDomain *domain) {
+  os << domain->getName();
+}
+
 /// Represents either a resolved availability domain or an identifier written
 /// in source that has not yet been resolved to a domain.
 class AvailabilityDomainOrIdentifier {

include/swift/AST/DiagnosticGroups.def

@@ -41,6 +41,7 @@
 GROUP(no_group, "")
 
 GROUP(ActorIsolatedCall, "actor-isolated-call")
+GROUP(AlwaysAvailableDomain, "always-available-domain")
 GROUP(AvailabilityUnrecognizedName, "availability-unrecognized-name")
 GROUP(ClangDeclarationImport, "clang-declaration-import")
 GROUP(ConformanceIsolation, "conformance-isolation")

include/swift/AST/DiagnosticsSema.def

@@ -7013,6 +7013,15 @@ ERROR(attr_availability_domain_not_usable_from_inline, none,
       "'@usableFromInline' or public",
       (AvailabilityDomain, DeclAttribute, const Decl *))
 
+GROUPED_WARNING(attr_availability_has_no_effect_domain_always_available,
+                AlwaysAvailableDomain, none,
+                "'%0' has no effect because '%1' is always available",
+                (DeclAttribute, AvailabilityDomain))
+GROUPED_WARNING(attr_availability_domain_always_available,
+                AlwaysAvailableDomain, none,
+                "'%0' is always available, use '*' instead",
+                (AvailabilityDomain))
+
 ERROR(availability_decl_unavailable, none,
       "%0 is unavailable%select{ in %2|}1%select{|: %3}3",
       (const ValueDecl *, bool, AvailabilityDomain, StringRef))
@@ -7134,6 +7143,12 @@ WARNING(availability_query_useless_enclosing_scope, none,
 NOTE(availability_query_useless_enclosing_scope_here, none,
      "enclosing scope here", ())
 
+GROUPED_WARNING(availability_query_useless_always_true,
+                AlwaysAvailableDomain, none,
+                "unnecessary check for '%0'; "
+                "this condition will always be %select{false|true}1",
+                (AvailabilityDomain, unsigned))
+
 ERROR(availability_decl_no_potential, none,
       "%kindonly0 cannot be marked potentially unavailable with '@available'",
       (const Decl *))

include/swift/AST/TypeCheckRequests.h

@@ -5505,7 +5505,6 @@ class AvailabilityDomainForDeclRequest
 private:
   friend SimpleRequest;
 
-  // Evaluation.
   std::optional<AvailabilityDomain> evaluate(Evaluator &evaluator,
                                              ValueDecl *decl) const;
 
@@ -5515,6 +5514,30 @@ class AvailabilityDomainForDeclRequest
   void cacheResult(std::optional<AvailabilityDomain> domain) const;
 };
 
+class IsCustomAvailabilityDomainPermanentlyEnabled
+    : public SimpleRequest<IsCustomAvailabilityDomainPermanentlyEnabled,
+                           bool(const CustomAvailabilityDomain *),
+                           RequestFlags::SeparatelyCached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  bool evaluate(Evaluator &evaluator,
+                const CustomAvailabilityDomain *customDomain) const;
+
+public:
+  bool isCached() const { return true; }
+  std::optional<bool> getCachedResult() const;
+  void cacheResult(bool isPermanentlyEnabled) const;
+
+  SourceLoc getNearestLoc() const {
+    auto *domain = std::get<0>(getStorage());
+    return extractNearestSourceLoc(domain->getDecl());
+  }
+};
+
 #define SWIFT_TYPEID_ZONE TypeChecker
 #define SWIFT_TYPEID_HEADER "swift/AST/TypeCheckerTypeIDZone.def"
 #include "swift/Basic/DefineTypeIDZone.h"

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -660,3 +660,7 @@ SWIFT_REQUEST(TypeChecker, ModuleHasTypeCheckerPerformanceHacksEnabledRequest,
 SWIFT_REQUEST(TypeChecker, AvailabilityDomainForDeclRequest,
               std::optional<AvailabilityDomain>(ValueDecl *),
               Cached | SplitCached, NoLocationInfo)
+
+SWIFT_REQUEST(TypeChecker, IsCustomAvailabilityDomainPermanentlyEnabled,
+              bool(const CustomAvailabilityDomain *),
+              SeparatelyCached, NoLocationInfo)

lib/AST/AvailabilityDomain.cpp

@@ -357,6 +357,45 @@ AvailabilityDomain::getRemappedDomain(const ASTContext &ctx,
   return *this;
 }
 
+bool IsCustomAvailabilityDomainPermanentlyEnabled::evaluate(
+    Evaluator &evaluator, const CustomAvailabilityDomain *customDomain) const {
+  switch (customDomain->getKind()) {
+  case CustomAvailabilityDomain::Kind::Enabled:
+  case CustomAvailabilityDomain::Kind::Disabled:
+  case CustomAvailabilityDomain::Kind::Dynamic:
+    return false;
+
+  case CustomAvailabilityDomain::Kind::AlwaysEnabled:
+    break;
+  }
+
+  auto *domainDecl = customDomain->getDecl();
+  if (!domainDecl)
+    return false;
+
+  if (auto deprecatedAttr = domainDecl->getDeprecatedAttr()) {
+    if (deprecatedAttr->getDomain().isUniversal())
+      return true;
+  }
+
+  if (auto unavailableAttr = domainDecl->getUnavailableAttr()) {
+    if (unavailableAttr->getDomain().isUniversal())
+      return true;
+  }
+
+  return false;
+}
+
+bool AvailabilityDomain::isPermanentlyAlwaysEnabled() const {
+  if (auto *customDomain = getCustomDomain()) {
+    if (auto *domainDecl = customDomain->getDecl())
+      return evaluateOrDefault(
+          domainDecl->getASTContext().evaluator,
+          IsCustomAvailabilityDomainPermanentlyEnabled{customDomain}, false);
+  }
+  return false;
+}
+
 void AvailabilityDomain::print(llvm::raw_ostream &os) const {
   os << getNameForAttributePrinting();
 }
@@ -408,8 +447,8 @@ CustomAvailabilityDomain::CustomAvailabilityDomain(Identifier name, Kind kind,
                                                    ModuleDecl *mod,
                                                    ValueDecl *decl,
                                                    FuncDecl *predicateFunc)
-    : name(name), kind(kind), mod(mod), decl(decl),
-      predicateFunc(predicateFunc) {
+    : name(name), mod(mod), decl(decl), predicateFunc(predicateFunc),
+      kind(kind) {
   ASSERT(!name.empty());
   ASSERT(mod);
   if (predicateFunc)

lib/AST/TypeCheckRequests.cpp

@@ -2867,3 +2867,23 @@ void AvailabilityDomainForDeclRequest::cacheResult(
 
   decl->getASTContext().evaluator.cacheNonEmptyOutput(*this, std::move(domain));
 }
+
+//----------------------------------------------------------------------------//
+// IsCustomAvailabilityDomainPermanentlyEnabled computation.
+//----------------------------------------------------------------------------//
+std::optional<bool>
+IsCustomAvailabilityDomainPermanentlyEnabled::getCachedResult() const {
+  auto *domain = std::get<0>(getStorage());
+
+  if (domain->flags.isPermanentlyEnabledComputed)
+    return domain->flags.isPermanentlyEnabled;
+  return std::nullopt;
+}
+
+void IsCustomAvailabilityDomainPermanentlyEnabled::cacheResult(
+    bool isPermanentlyEnabled) const {
+  auto *domain = const_cast<CustomAvailabilityDomain *>(std::get<0>(getStorage()));
+
+  domain->flags.isPermanentlyEnabledComputed = true;
+  domain->flags.isPermanentlyEnabled = isPermanentlyEnabled;
+}

lib/Sema/TypeCheckAccess.cpp

@@ -2281,12 +2281,45 @@ class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {
   void checkAvailabilityDomains(const Decl *D) {
     D = D->getAbstractSyntaxDeclForAttributes();
 
+    auto &ctx = D->getASTContext();
     auto where = Where.withReason(ExportabilityReason::AvailableAttribute);
     for (auto attr : D->getSemanticAvailableAttrs()) {
-      if (auto *domainDecl = attr.getDomain().getDecl()) {
+      auto domain = attr.getDomain();
+      if (auto *domainDecl = domain.getDecl()) {
         diagnoseDeclAvailability(domainDecl,
                                  attr.getParsedAttr()->getDomainLoc(), nullptr,
                                  where, std::nullopt);
+
+        if (!attr.isVersionSpecific()) {
+          // Check whether the availability domain is permanently enabled. If it
+          // is, suggest modifying or removing the attribute.
+          if (domain.isPermanentlyAlwaysEnabled()) {
+            auto parsedAttr = attr.getParsedAttr();
+            switch (parsedAttr->getKind()) {
+            case AvailableAttr::Kind::Default:
+              // This introduction constraint has no effect since it will never
+              // restrict use of the declaration. Provide a fix-it remove the
+              // attribute.
+              diagnoseAndRemoveAttr(
+                  D, attr.getParsedAttr(),
+                  diag::attr_availability_has_no_effect_domain_always_available,
+                  parsedAttr, domain);
+              break;
+            case AvailableAttr::Kind::Deprecated:
+            case AvailableAttr::Kind::NoAsync:
+            case AvailableAttr::Kind::Unavailable:
+              // Any other kind of constraint always constrains use of the
+              // declaration, so provide a fix-it to specify `*` instead of the
+              // custom domain.
+              ctx.Diags
+                  .diagnose(parsedAttr->getDomainLoc(),
+                            diag::attr_availability_domain_always_available,
+                            domain)
+                  .fixItReplace(parsedAttr->getDomainLoc(), "*");
+              break;
+            }
+          }
+        }
       }
     }
   }

lib/Sema/TypeCheckStmt.cpp

@@ -810,6 +810,37 @@ ConcreteDeclRef TypeChecker::getReferencedDeclForHasSymbolCondition(Expr *E) {
   return ConcreteDeclRef();
 }
 
+static bool typeCheckAvailableStmtConditionElement(StmtConditionElement &elt,
+                                                   bool &isFalseable,
+                                                   DeclContext *dc) {
+  auto info = elt.getAvailability();
+  if (info->isInvalid()) {
+    isFalseable = true;
+    return false;
+  }
+
+  auto &diags = dc->getASTContext().Diags;
+  bool isConditionAlwaysTrue = false;
+
+  if (auto query = info->getAvailabilityQuery()) {
+    auto domain = query->getDomain();
+    if (query->isConstant() && domain.isPermanentlyAlwaysEnabled()) {
+      isConditionAlwaysTrue = *query->getConstantResult();
+
+      diags
+        .diagnose(elt.getStartLoc(),
+                  diag::availability_query_useless_always_true, domain,
+                  isConditionAlwaysTrue)
+        .highlight(elt.getSourceRange());
+    }
+  }
+
+  if (!isConditionAlwaysTrue)
+    isFalseable = true;
+
+  return false;
+}
+
 static bool typeCheckHasSymbolStmtConditionElement(StmtConditionElement &elt,
                                                    DeclContext *dc) {
   auto Info = elt.getHasSymbolInfo();
@@ -895,8 +926,7 @@ bool TypeChecker::typeCheckStmtConditionElement(StmtConditionElement &elt,
                                                 DeclContext *dc) {
   switch (elt.getKind()) {
   case StmtConditionElement::CK_Availability:
-    isFalsable = true;
-    return false;
+    return typeCheckAvailableStmtConditionElement(elt, isFalsable, dc);
   case StmtConditionElement::CK_HasSymbol:
     isFalsable = true;
     return typeCheckHasSymbolStmtConditionElement(elt, dc);
@@ -924,8 +954,9 @@ static bool typeCheckConditionForStatement(LabeledConditionalStmt *stmt,
         TypeChecker::typeCheckStmtConditionElement(elt, hadAnyFalsable, dc);
   }
 
-  // If the binding is not refutable, and there *is* an else, reject it as
-  // unreachable.
+  // If none of the statement's conditions can be false, diagnose.
+  // FIXME: Also diagnose if none of the statements conditions can be true.
+  // FIXME: Offer a fix-it to remove the unreachable code.
   if (!hadAnyFalsable && !hadError) {
     auto &diags = dc->getASTContext().Diags;
     Diag<> msg = diag::invalid_diagnostic;