Merge pull request swiftlang#79494 from tshortli/type-check-availability-spec

Parse/Sema: Move some `AvailabilitySpec` diagnostics from Parse to Sema

Author: tshortli

include/swift/AST/Attr.h

@@ -151,7 +151,7 @@ class DeclAttribute : public AttributeBase {
       Value : 32
     );
 
-    SWIFT_INLINE_BITFIELD(AvailableAttr, DeclAttribute, 4+1+1+1+1,
+    SWIFT_INLINE_BITFIELD(AvailableAttr, DeclAttribute, 4+1+1+1+1+1+1,
       /// An `AvailableAttr::Kind` value.
       Kind : 4,
 
@@ -163,7 +163,15 @@ class DeclAttribute : public AttributeBase {
       HasRenamedDecl : 1,
 
       /// Whether this attribute was spelled `@_spi_available`.
-      IsSPI : 1
+      IsSPI : 1,
+
+      /// Whether this attribute is an interior attribute of a group of
+      /// `@available` attributes that were written in source using short form
+      /// syntax (`@available(macOS 15, ...)`).
+      IsFollowedByGroupedAvailableAttr : 1,
+
+      /// Whether this attribute was followed by `, *` when parsed from source.
+      IsFollowedByWildcard : 1
     );
 
     SWIFT_INLINE_BITFIELD(ClangImporterSynthesizedTypeAttr, DeclAttribute, 1,
@@ -805,6 +813,10 @@ class AvailableAttr : public DeclAttribute {
   /// a rename decl even when this string is empty.
   StringRef getRename() const { return Rename; }
 
+  bool hasCachedRenamedDecl() const {
+    return Bits.AvailableAttr.HasRenamedDecl;
+  }
+
   /// Whether this is an unconditionally unavailable entity.
   bool isUnconditionallyUnavailable() const;
 
@@ -817,6 +829,28 @@ class AvailableAttr : public DeclAttribute {
   /// Whether this attribute was spelled `@_spi_available`.
   bool isSPI() const { return Bits.AvailableAttr.IsSPI; }
 
+  /// Returns the following `@available` if this was generated from an
+  /// attribute that was written in source using short form syntax, e.g.
+  /// `@available(macOS 15, iOS 18, *)`.
+  const AvailableAttr *getNextGroupedAvailableAttr() const {
+    if (Bits.AvailableAttr.IsFollowedByGroupedAvailableAttr)
+      return dyn_cast_or_null<AvailableAttr>(Next);
+    return nullptr;
+  }
+
+  void setIsFollowedByGroupedAvailableAttr() {
+    Bits.AvailableAttr.IsFollowedByGroupedAvailableAttr = true;
+  }
+
+  /// Whether this attribute was followed by `, *` when parsed from source.
+  bool isFollowedByWildcard() const {
+    return Bits.AvailableAttr.IsFollowedByWildcard;
+  }
+
+  void setIsFollowedByWildcard() {
+    Bits.AvailableAttr.IsFollowedByWildcard = true;
+  }
+
   /// Returns the kind of availability the attribute specifies.
   Kind getKind() const { return static_cast<Kind>(Bits.AvailableAttr.Kind); }
 
@@ -860,10 +894,6 @@ class AvailableAttr : public DeclAttribute {
     return DA->getKind() == DeclAttrKind::Available;
   }
 
-  bool hasCachedRenamedDecl() const {
-    return Bits.AvailableAttr.HasRenamedDecl;
-  }
-
 private:
   friend class RenamedDeclRequest;
 

include/swift/AST/DiagnosticsParse.def

@@ -1998,18 +1998,6 @@ ERROR(availability_query_wildcard_required, none,
 ERROR(unavailability_query_wildcard_not_required, none,
       "platform wildcard '*' is always implicit in #unavailable", ())
 
-ERROR(availability_must_occur_alone, none,
-      "'%0' version-availability must be specified alone", (StringRef))
-
-ERROR(pound_available_swift_not_allowed, none,
-      "Swift language version checks not allowed in %0(...)", (StringRef))
-
-ERROR(pound_available_package_description_not_allowed, none,
-      "PackageDescription version checks not allowed in %0(...)", (StringRef))
-
-ERROR(availability_query_repeated_platform, none,
-      "version for '%0' already specified", (StringRef))
-
 ERROR(availability_cannot_be_mixed,none,
       "#available and #unavailable cannot be in the same statement", ())
 

include/swift/AST/DiagnosticsSema.def

@@ -6963,6 +6963,22 @@ ERROR(conformance_availability_only_version_newer, none,
       "conformance of %0 to %1 is only available in %2 %3 or newer",
       (Type, Type, StringRef, llvm::VersionTuple))
 
+ERROR(availability_must_occur_alone, none,
+      "'%0' version-availability must be specified alone", (StringRef))
+
+//------------------------------------------------------------------------------
+// MARK: if #available(...)
+//------------------------------------------------------------------------------
+
+ERROR(availability_query_swift_not_allowed, none,
+      "Swift language version checks not allowed in %0(...)", (StringRef))
+
+ERROR(availability_query_package_description_not_allowed, none,
+      "PackageDescription version checks not allowed in %0(...)", (StringRef))
+
+ERROR(availability_query_repeated_platform, none,
+      "version for '%0' already specified", (StringRef))
+
 //------------------------------------------------------------------------------
 // MARK: @discardableResult
 //------------------------------------------------------------------------------

lib/AST/Attr.cpp

@@ -2141,6 +2141,8 @@ AvailableAttr::AvailableAttr(
       ObsoletedRange(ObsoletedRange) {
   Bits.AvailableAttr.Kind = static_cast<uint8_t>(Kind);
   Bits.AvailableAttr.IsSPI = IsSPI;
+  Bits.AvailableAttr.IsFollowedByGroupedAvailableAttr = false;
+  Bits.AvailableAttr.IsFollowedByWildcard = false;
 }
 
 AvailableAttr *AvailableAttr::createUniversallyUnavailable(ASTContext &C,

lib/Parse/ParseDecl.cpp

@@ -790,24 +790,36 @@ bool Parser::parseAvailability(
     // we will synthesize
     //   @available(_PackageDescription, introduced: 4.2)
 
+    AvailabilitySpec *PrevSpec = nullptr;
     for (auto *Spec : Specs) {
-      // FIXME: [availability] Allow arbitrary availability domains.
+      if (Spec->isWildcard())
+        continue;
+
       std::optional<AvailabilityDomain> Domain = Spec->getDomain();
       if (!Domain)
         continue;
 
-      addAttribute(new (Context) AvailableAttr(
-          AtLoc, attrRange, *Domain, Spec->getSourceRange().Start,
-          AvailableAttr::Kind::Default,
-          /*Message=*/StringRef(),
-          /*Rename=*/StringRef(),
-          /*Introduced=*/Spec->getVersion(),
-          /*IntroducedRange=*/Spec->getVersionSrcRange(),
-          /*Deprecated=*/llvm::VersionTuple(),
-          /*DeprecatedRange=*/SourceRange(),
-          /*Obsoleted=*/llvm::VersionTuple(),
-          /*ObsoletedRange=*/SourceRange(),
-          /*Implicit=*/false, AttrName == SPI_AVAILABLE_ATTRNAME));
+      auto Attr = new (Context)
+          AvailableAttr(AtLoc, attrRange, *Domain, Spec->getSourceRange().Start,
+                        AvailableAttr::Kind::Default,
+                        /*Message=*/StringRef(),
+                        /*Rename=*/StringRef(),
+                        /*Introduced=*/Spec->getVersion(),
+                        /*IntroducedRange=*/Spec->getVersionSrcRange(),
+                        /*Deprecated=*/llvm::VersionTuple(),
+                        /*DeprecatedRange=*/SourceRange(),
+                        /*Obsoleted=*/llvm::VersionTuple(),
+                        /*ObsoletedRange=*/SourceRange(),
+                        /*Implicit=*/false, AttrName == SPI_AVAILABLE_ATTRNAME);
+      addAttribute(Attr);
+
+      if (PrevSpec) {
+        if (PrevSpec->isWildcard())
+          Attr->setIsFollowedByWildcard();
+        else
+          Attr->setIsFollowedByGroupedAvailableAttr();
+      }
+      PrevSpec = Spec;
     }
 
     return true;

lib/Parse/ParseStmt.cpp

@@ -1289,9 +1289,8 @@ static void parseGuardedPattern(Parser &P, GuardedPattern &result,
 /// removing specs for unrecognized platforms.
 static void
 validateAvailabilitySpecList(Parser &P,
-                             SmallVectorImpl<AvailabilitySpec *> &Specs,
+                             const SmallVectorImpl<AvailabilitySpec *> &Specs,
                              Parser::AvailabilitySpecSource Source) {
-  llvm::SmallSet<PlatformKind, 4> Platforms;
   std::optional<SourceLoc> WildcardSpecLoc = std::nullopt;
 
   if (Specs.size() == 1) {
@@ -1303,37 +1302,9 @@ validateAvailabilitySpecList(Parser &P,
       return;
   }
 
-  SmallVector<AvailabilitySpec *, 5> RecognizedSpecs;
   for (auto *Spec : Specs) {
-    RecognizedSpecs.push_back(Spec);
     if (Spec->isWildcard()) {
       WildcardSpecLoc = Spec->getStartLoc();
-      continue;
-    }
-
-    // We keep specs for unrecognized domains around for error recovery
-    // during parsing but remove them once parsing is completed.
-    auto Domain = Spec->getDomain();
-    if (!Domain) {
-      RecognizedSpecs.pop_back();
-      continue;
-    }
-
-    if (!Domain->isPlatform()) {
-      P.diagnose(Spec->getStartLoc(), diag::availability_must_occur_alone,
-                 Domain->getNameForAttributePrinting());
-      continue;
-    }
-
-    bool Inserted = Platforms.insert(Spec->getPlatform()).second;
-    if (!Inserted) {
-      // Rule out multiple version specs referring to the same platform.
-      // For example, we emit an error for
-      /// #available(OSX 10.10, OSX 10.11, *)
-      PlatformKind Platform = Spec->getPlatform();
-      P.diagnose(Spec->getStartLoc(),
-                 diag::availability_query_repeated_platform,
-                 platformString(Platform));
     }
   }
 
@@ -1362,8 +1333,6 @@ validateAvailabilitySpecList(Parser &P,
     break;
   }
   }
-
-  Specs = RecognizedSpecs;
 }
 
 // #available(...)
@@ -1398,18 +1367,6 @@ ParserResult<PoundAvailableInfo> Parser::parseStmtConditionPoundAvailable() {
   SmallVector<AvailabilitySpec *, 5> Specs;
   ParserStatus Status = parseAvailabilitySpecList(Specs, Source);
 
-  for (auto *Spec : Specs) {
-    if (Spec->getPlatform() != PlatformKind::none || Spec->isWildcard())
-      continue;
-
-    diagnose(Spec->getStartLoc(),
-             Spec->getDomain()->isSwiftLanguage()
-                 ? diag::pound_available_swift_not_allowed
-                 : diag::pound_available_package_description_not_allowed,
-             getTokenText(MainToken));
-    Status.setIsParseError();
-  }
-
   SourceLoc RParenLoc;
   if (parseMatchingToken(tok::r_paren, RParenLoc,
                          diag::avail_query_expected_rparen, LParenLoc))

lib/Sema/MiscDiagnostics.cpp

@@ -5123,17 +5123,44 @@ checkImplicitPromotionsInCondition(const StmtConditionElement &cond,
 /// was emitted.
 static bool diagnoseAvailabilityCondition(PoundAvailableInfo *info,
                                           DeclContext *DC) {
+  auto &diags = DC->getASTContext().Diags;
+
+  llvm::SmallSet<AvailabilityDomain, 8> seenDomains;
+  for (auto spec : info->getQueries()) {
+    auto domain = spec->getDomain();
+    if (!domain) {
+      continue;
+    }
+
+    if (!domain->isPlatform()) {
+      diags.diagnose(
+          spec->getStartLoc(),
+          domain->isSwiftLanguage()
+              ? diag::availability_query_swift_not_allowed
+              : diag::availability_query_package_description_not_allowed,
+          info->isUnavailability() ? "#unavailable" : "#available");
+      return true;
+    }
+
+    // Diagnose duplicate platforms.
+    if (!seenDomains.insert(*domain).second) {
+      diags.diagnose(spec->getStartLoc(),
+                     diag::availability_query_repeated_platform,
+                     domain->getNameForAttributePrinting());
+      return true;
+    }
+  }
+
   // Reject inlinable code using availability macros. In order to lift this
   // restriction, macros would need to either be expanded when printed in
   // swiftinterfaces or be parsable as macros by module clients.
   auto fragileKind = DC->getFragileFunctionKind();
   if (fragileKind.kind != FragileFunctionKind::None) {
     for (auto availSpec : info->getQueries()) {
       if (availSpec->getMacroLoc().isValid()) {
-        DC->getASTContext().Diags.diagnose(
-            availSpec->getMacroLoc(),
-            swift::diag::availability_macro_in_inlinable,
-            fragileKind.getSelector());
+        diags.diagnose(availSpec->getMacroLoc(),
+                       swift::diag::availability_macro_in_inlinable,
+                       fragileKind.getSelector());
         return true;
       }
     }

lib/Sema/TypeCheckAttr.cpp

@@ -4957,20 +4957,73 @@ void AttributeChecker::checkOriginalDefinedInAttrs(
   }
 }
 
-void AttributeChecker::checkAvailableAttrs(ArrayRef<AvailableAttr *> Attrs) {
-  if (Attrs.empty())
+/// Find each of the `AvailableAttr`s that represents the first attribute in a
+/// group of attributes what were parsed from a short-form available attribute,
+/// e.g. `@available(macOS , iOS, *)`.
+static llvm::SmallSet<const AvailableAttr *, 8>
+getAvailableAttrGroups(ArrayRef<AvailableAttr *> attrs) {
+  llvm::SmallSet<const AvailableAttr *, 8> heads;
+
+  // Find each attribute that belongs to a group.
+  for (auto attr : attrs) {
+    if (attr->getNextGroupedAvailableAttr())
+      heads.insert(attr);
+  }
+
+  // Remove the interior attributes of each group, leaving only the head.
+  for (auto attr : attrs) {
+    if (auto next = attr->getNextGroupedAvailableAttr()) {
+      heads.erase(next);
+    }
+  }
+
+  return heads;
+}
+
+void AttributeChecker::checkAvailableAttrs(ArrayRef<AvailableAttr *> attrs) {
+  if (attrs.empty())
     return;
 
   // Only diagnose top level decls since nested ones may have inherited availability.
   if (!D->getDeclContext()->getInnermostDeclarationDeclContext()) {
     // If all available are spi available, we should use @_spi instead.
-    if (std::all_of(Attrs.begin(), Attrs.end(), [](AvailableAttr *AV) {
-      return AV->isSPI();
-    })) {
+    if (std::all_of(attrs.begin(), attrs.end(),
+                    [](AvailableAttr *AV) { return AV->isSPI(); })) {
       diagnose(D->getLoc(), diag::spi_preferred_over_spi_available);
     }
   }
 
+  auto attrGroups = getAvailableAttrGroups(attrs);
+  for (const AvailableAttr *groupHead : attrGroups) {
+    llvm::SmallSet<AvailabilityDomain, 8> seenDomains;
+
+    for (auto *groupedAttr = groupHead; groupedAttr != nullptr;
+         groupedAttr = groupedAttr->getNextGroupedAvailableAttr()) {
+      auto loc = groupedAttr->getLocation();
+      auto attr = D->getSemanticAvailableAttr(groupedAttr);
+
+      // If the attribute cannot be resolved, it may have had an unrecognized
+      // domain. Assume this unrecognized domain could be an unrecognized
+      // platform and skip it.
+      if (!attr)
+        continue;
+
+      // Only platform availability is allowed to be written in short form.
+      auto domain = attr->getDomain();
+      if (!domain.isPlatform()) {
+        diagnose(loc, diag::availability_must_occur_alone,
+                 domain.getNameForAttributePrinting());
+        continue;
+      }
+
+      // Diagnose duplicate platforms.
+      if (!seenDomains.insert(domain).second) {
+        diagnose(loc, diag::availability_query_repeated_platform,
+                 domain.getNameForAttributePrinting());
+      }
+    }
+  }
+
   if (Ctx.LangOpts.DisableAvailabilityChecking)
     return;
 

test/Parse/availability_query.swift

@@ -47,6 +47,7 @@ if #available(OSX 51 { // expected-error {{expected ')'}} expected-note {{to mat
 
 if #available(iDishwasherOS 51) { // expected-warning {{unrecognized platform name 'iDishwasherOS'}}
 // expected-error@-1 {{must handle potential future platforms with '*'}}
+// expected-error@-2 {{condition required for target platform}}
 }
 
 if #available(iDishwasherOS 51, *) { // expected-warning {{unrecognized platform name 'iDishwasherOS'}}
@@ -125,4 +126,7 @@ if let _ = Optional(42), #available(iOS 8.0, *) {}
 if #available(macOS 51, *) {
 }
 
+// FIXME: This is weird, but it's already accepted. It should probably be diagnosed.
+if #available(*, macOS 51) {
+}