Merge pull request #79572 from tshortli/wildcard-availability-diags

tshortli · web-flow · commit e83f73106c09 · 2025-02-24T08:41:36.000-08:00
Parse/Sema: Move `#available` query wildcard diagnostics to Sema
diff --git a/include/swift/AST/Attr.h b/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+1+1,
+    SWIFT_INLINE_BITFIELD(AvailableAttr, DeclAttribute, 4+1+1+1+1+1+1+1,
       /// An `AvailableAttr::Kind` value.
       Kind : 4,
 
@@ -165,13 +165,14 @@ class DeclAttribute : public AttributeBase {
       /// Whether this attribute was spelled `@_spi_available`.
       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 belongs to a chain of adjacent `@available` attributes that were generated from a single attribute written in source using short form syntax e.g. (`@available(macOS 15, iOS 18, *)`).
+      IsGroupMember : 1,
 
-      /// Whether this attribute was followed by `, *` when parsed from source.
-      IsFollowedByWildcard : 1
+      /// Whether this attribute is the final one in its group.
+      IsGroupTerminator : 1,
+
+      /// Whether this attribute's specification was followed by `, *` in source.
+      IsAdjacentToWildcard : 1
     );
 
     SWIFT_INLINE_BITFIELD(ClangImporterSynthesizedTypeAttr, DeclAttribute, 1,
@@ -829,26 +830,30 @@ 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, *)`.
+  /// Returns the next attribute in the chain of adjacent `@available`
+  /// attributes that were generated from a single attribute written in source
+  /// using short form syntax e.g. (`@available(macOS 15, iOS 18, *)`).
   const AvailableAttr *getNextGroupedAvailableAttr() const {
-    if (Bits.AvailableAttr.IsFollowedByGroupedAvailableAttr)
+    if (Bits.AvailableAttr.IsGroupMember && !isGroupTerminator())
       return dyn_cast_or_null<AvailableAttr>(Next);
     return nullptr;
   }
 
-  void setIsFollowedByGroupedAvailableAttr() {
-    Bits.AvailableAttr.IsFollowedByGroupedAvailableAttr = true;
-  }
+  bool isGroupMember() const { return Bits.AvailableAttr.IsGroupMember; }
+  void setIsGroupMember() { Bits.AvailableAttr.IsGroupMember = true; }
 
-  /// Whether this attribute was followed by `, *` when parsed from source.
-  bool isFollowedByWildcard() const {
-    return Bits.AvailableAttr.IsFollowedByWildcard;
+  /// Whether this attribute is the final one in its group.
+  bool isGroupTerminator() const {
+    return Bits.AvailableAttr.IsGroupTerminator;
   }
+  void setIsGroupTerminator() { Bits.AvailableAttr.IsGroupTerminator = true; }
 
-  void setIsFollowedByWildcard() {
-    Bits.AvailableAttr.IsFollowedByWildcard = true;
+  /// Whether this attribute's specification was followed by `, *` in source.
+  bool isAdjacentToWildcard() const {
+    return Bits.AvailableAttr.IsAdjacentToWildcard;
+  }
+  void setIsAdjacentToWildcard() {
+    Bits.AvailableAttr.IsAdjacentToWildcard = true;
   }
 
   /// Returns the kind of availability the attribute specifies.
diff --git a/include/swift/AST/DiagnosticsParse.def b/include/swift/AST/DiagnosticsParse.def
@@ -1992,12 +1992,6 @@ NOTE(avail_query_meant_introduced,PointsToFirstBadToken,
 ERROR(avail_query_version_comparison_not_needed,
       none,"version comparison not needed", ())
 
-ERROR(availability_query_wildcard_required, none,
-      "must handle potential future platforms with '*'", ())
-
-ERROR(unavailability_query_wildcard_not_required, none,
-      "platform wildcard '*' is always implicit in #unavailable", ())
-
 ERROR(availability_cannot_be_mixed,none,
       "#available and #unavailable cannot be in the same statement", ())
 
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -6990,6 +6990,12 @@ ERROR(availability_query_package_description_not_allowed, none,
 ERROR(availability_query_repeated_platform, none,
       "version for '%0' already specified", (StringRef))
 
+ERROR(availability_query_wildcard_required, none,
+      "must handle potential future platforms with '*'", ())
+
+ERROR(unavailability_query_wildcard_not_required, none,
+      "platform wildcard '*' is always implicit in #unavailable", ())
+
 //------------------------------------------------------------------------------
 // MARK: @discardableResult
 //------------------------------------------------------------------------------
diff --git a/include/swift/AST/Stmt.h b/include/swift/AST/Stmt.h
@@ -492,28 +492,36 @@ class alignas(8) PoundAvailableInfo final :
   /// This is filled in by Sema.
   VersionRange VariantAvailableRange;
 
-  /// Indicates that the expression is checking if a version range 
-  /// is **not** available.
-  bool _isUnavailability;
+  struct {
+    unsigned isInvalid : 1;
+
+    /// Indicates that the expression is checking if a version range
+    /// is **not** available.
+    unsigned isUnavailability : 1;
+  } Flags;
 
   PoundAvailableInfo(SourceLoc PoundLoc, SourceLoc LParenLoc,
                      ArrayRef<AvailabilitySpec *> queries, SourceLoc RParenLoc,
                      bool isUnavailability)
-   : PoundLoc(PoundLoc), LParenLoc(LParenLoc), RParenLoc(RParenLoc),
-     NumQueries(queries.size()), AvailableRange(VersionRange::empty()),
-     VariantAvailableRange(VersionRange::empty()), 
-     _isUnavailability(isUnavailability) {
+      : PoundLoc(PoundLoc), LParenLoc(LParenLoc), RParenLoc(RParenLoc),
+        NumQueries(queries.size()), AvailableRange(VersionRange::empty()),
+        VariantAvailableRange(VersionRange::empty()), Flags() {
+    Flags.isInvalid = false;
+    Flags.isUnavailability = isUnavailability;
     std::uninitialized_copy(queries.begin(), queries.end(),
                             getTrailingObjects<AvailabilitySpec *>());
   }
-  
+
 public:
   static PoundAvailableInfo *create(ASTContext &ctx, SourceLoc PoundLoc,
                                     SourceLoc LParenLoc,
                                     ArrayRef<AvailabilitySpec *> queries,
                                     SourceLoc RParenLoc,
                                     bool isUnavailability);
-  
+
+  bool isInvalid() const { return Flags.isInvalid; }
+  void setInvalid() { Flags.isInvalid = true; }
+
   ArrayRef<AvailabilitySpec *> getQueries() const {
     return llvm::ArrayRef(getTrailingObjects<AvailabilitySpec *>(), NumQueries);
   }
@@ -541,7 +549,7 @@ class alignas(8) PoundAvailableInfo final :
     VariantAvailableRange = Range;
   }
 
-  bool isUnavailability() const { return _isUnavailability; }
+  bool isUnavailability() const { return Flags.isUnavailability; }
 };
 
 /// An expression that guards execution based on whether the symbols for the
diff --git a/lib/AST/Attr.cpp b/lib/AST/Attr.cpp
@@ -2144,8 +2144,9 @@ AvailableAttr::AvailableAttr(
       ObsoletedRange(ObsoletedRange) {
   Bits.AvailableAttr.Kind = static_cast<uint8_t>(Kind);
   Bits.AvailableAttr.IsSPI = IsSPI;
-  Bits.AvailableAttr.IsFollowedByGroupedAvailableAttr = false;
-  Bits.AvailableAttr.IsFollowedByWildcard = false;
+  Bits.AvailableAttr.IsGroupMember = false;
+  Bits.AvailableAttr.IsGroupTerminator = false;
+  Bits.AvailableAttr.IsAdjacentToWildcard = false;
 }
 
 AvailableAttr *AvailableAttr::createUniversallyUnavailable(ASTContext &C,
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
@@ -790,10 +790,13 @@ bool Parser::parseAvailability(
     // we will synthesize
     //   @available(_PackageDescription, introduced: 4.2)
 
-    AvailabilitySpec *PrevSpec = nullptr;
+    AvailableAttr *PrevAttr = nullptr;
     for (auto *Spec : Specs) {
-      if (Spec->isWildcard())
+      if (Spec->isWildcard()) {
+        if (PrevAttr)
+          PrevAttr->setIsAdjacentToWildcard();
         continue;
+      }
 
       std::optional<AvailabilityDomain> Domain = Spec->getDomain();
       if (!Domain)
@@ -813,13 +816,11 @@ bool Parser::parseAvailability(
                         /*Implicit=*/false, AttrName == SPI_AVAILABLE_ATTRNAME);
       addAttribute(Attr);
 
-      if (PrevSpec) {
-        if (PrevSpec->isWildcard())
-          Attr->setIsFollowedByWildcard();
-        else
-          Attr->setIsFollowedByGroupedAvailableAttr();
-      }
-      PrevSpec = Spec;
+      Attr->setIsGroupMember();
+      if (!PrevAttr)
+        Attr->setIsGroupTerminator();
+
+      PrevAttr = Attr;
     }
 
     return true;
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
@@ -1292,48 +1292,18 @@ static void
 validateAvailabilitySpecList(Parser &P,
                              const SmallVectorImpl<AvailabilitySpec *> &Specs,
                              Parser::AvailabilitySpecSource Source) {
-  std::optional<SourceLoc> WildcardSpecLoc = std::nullopt;
-
-  if (Specs.size() == 1) {
-    // @available(swift N) and @available(_PackageDescription N) are allowed
-    // only in isolation; they cannot be combined with other availability specs
-    // in a single list.
-    auto domain = Specs[0]->getDomain();
-    if (domain && !domain->isPlatform())
-      return;
-  }
+  if (Source != Parser::AvailabilitySpecSource::Macro)
+    return;
 
+  std::optional<SourceLoc> WildcardSpecLoc;
   for (auto *Spec : Specs) {
     if (Spec->isWildcard()) {
       WildcardSpecLoc = Spec->getStartLoc();
     }
   }
 
-  switch (Source) {
-  case Parser::AvailabilitySpecSource::Available: {
-    if (WildcardSpecLoc == std::nullopt) {
-      SourceLoc InsertWildcardLoc = P.PreviousLoc;
-      P.diagnose(InsertWildcardLoc, diag::availability_query_wildcard_required)
-        .fixItInsertAfter(InsertWildcardLoc, ", *");
-    }
-    break;
-  }
-  case Parser::AvailabilitySpecSource::Unavailable: {
-    if (WildcardSpecLoc != std::nullopt) {
-      SourceLoc Loc = WildcardSpecLoc.value();
-      P.diagnose(Loc, diag::unavailability_query_wildcard_not_required)
-        .fixItRemove(Loc);
-    }
-    break;
-  }
-  case Parser::AvailabilitySpecSource::Macro: {
-    if (WildcardSpecLoc != std::nullopt) {
-      SourceLoc Loc = WildcardSpecLoc.value();
-      P.diagnose(Loc, diag::attr_availability_wildcard_in_macro);
-    }
-    break;
-  }
-  }
+  if (WildcardSpecLoc)
+    P.diagnose(*WildcardSpecLoc, diag::attr_availability_wildcard_in_macro);
 }
 
 // #available(...)
@@ -1387,6 +1357,9 @@ ParserResult<PoundAvailableInfo> Parser::parseStmtConditionPoundAvailable() {
 
   auto *result = PoundAvailableInfo::create(Context, PoundLoc, LParenLoc, Specs,
                                             RParenLoc, isUnavailability);
+  if (Status.isError())
+    result->setInvalid();
+
   return makeParserResult(Status, result);
 }
 
diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp
@@ -5123,16 +5123,22 @@ checkImplicitPromotionsInCondition(const StmtConditionElement &cond,
 /// was emitted.
 static bool diagnoseAvailabilityCondition(PoundAvailableInfo *info,
                                           DeclContext *DC) {
+  if (info->isInvalid())
+    return false;
+
   auto &diags = DC->getASTContext().Diags;
   StringRef queryName =
       info->isUnavailability() ? "#unavailable" : "#available";
 
+  std::optional<SourceLoc> wildcardLoc;
   llvm::SmallSet<AvailabilityDomain, 8> seenDomains;
   for (auto spec : info->getSemanticAvailabilitySpecs(DC)) {
-    if (spec.isWildcard())
+    auto parsedSpec = spec.getParsedSpec();
+    if (spec.isWildcard()) {
+      wildcardLoc = parsedSpec->getStartLoc();
       continue;
+    }
 
-    auto parsedSpec = spec.getParsedSpec();
     auto domain = spec.getDomain();
 
     if (!domain.isPlatform()) {
@@ -5154,6 +5160,21 @@ static bool diagnoseAvailabilityCondition(PoundAvailableInfo *info,
     }
   }
 
+  if (info->isUnavailability()) {
+    if (wildcardLoc) {
+      diags
+          .diagnose(*wildcardLoc,
+                    diag::unavailability_query_wildcard_not_required)
+          .fixItRemove(*wildcardLoc);
+    }
+  } else if (!wildcardLoc) {
+    if (info->getQueries().size() > 0) {
+      auto insertLoc = info->getQueries().back()->getSourceRange().End;
+      diags.diagnose(insertLoc, diag::availability_query_wildcard_required)
+          .fixItInsertAfter(insertLoc, ", *");
+    }
+  }
+
   // 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.
@@ -5260,7 +5281,8 @@ static void checkLabeledStmtConditions(ASTContext &ctx,
       break;
     case StmtConditionElement::CK_Availability: {
       auto info = elt.getAvailability();
-      (void)diagnoseAvailabilityCondition(info, DC);
+      if (diagnoseAvailabilityCondition(info, DC))
+        info->setInvalid();
       break;
     }
     case StmtConditionElement::CK_HasSymbol: {
diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp
@@ -4942,7 +4942,7 @@ getAvailableAttrGroups(ArrayRef<AvailableAttr *> attrs) {
 
   // Find each attribute that belongs to a group.
   for (auto attr : attrs) {
-    if (attr->getNextGroupedAvailableAttr())
+    if (attr->isGroupMember())
       heads.insert(attr);
   }
 
@@ -4973,9 +4973,18 @@ void AttributeChecker::checkAvailableAttrs(ArrayRef<AvailableAttr *> attrs) {
   for (const AvailableAttr *groupHead : attrGroups) {
     llvm::SmallSet<AvailabilityDomain, 8> seenDomains;
 
+    SourceLoc groupEndLoc;
+    bool requiresWildcard = false;
+    bool foundWildcard = false;
+    int groupAttrCount = 0;
     for (auto *groupedAttr = groupHead; groupedAttr != nullptr;
          groupedAttr = groupedAttr->getNextGroupedAvailableAttr()) {
+      groupAttrCount++;
       auto loc = groupedAttr->getLocation();
+      groupEndLoc = groupedAttr->getEndLoc();
+      if (groupedAttr->isAdjacentToWildcard())
+        foundWildcard = true;
+
       auto attr = D->getSemanticAvailableAttr(groupedAttr);
 
       // If the attribute cannot be resolved, it may have had an unrecognized
@@ -4984,12 +4993,19 @@ void AttributeChecker::checkAvailableAttrs(ArrayRef<AvailableAttr *> attrs) {
       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;
+      if (domain.isPlatform())
+        requiresWildcard = true;
+
+      if (groupAttrCount > 1 || groupedAttr->isAdjacentToWildcard() ||
+          !groupedAttr->isGroupTerminator()) {
+        // Only platform availability is allowed to be written groups with more
+        // than one member.
+        if (!domain.isPlatform()) {
+          diagnose(loc, diag::availability_must_occur_alone,
+                   domain.getNameForAttributePrinting());
+          continue;
+        }
       }
 
       // Diagnose duplicate platforms.
@@ -4998,6 +5014,11 @@ void AttributeChecker::checkAvailableAttrs(ArrayRef<AvailableAttr *> attrs) {
                  domain.getNameForAttributePrinting());
       }
     }
+
+    if (requiresWildcard && !foundWildcard) {
+      diagnose(groupEndLoc, diag::availability_query_wildcard_required)
+          .fixItInsert(groupEndLoc, ", *");
+    }
   }
 
   if (Ctx.LangOpts.DisableAvailabilityChecking)
diff --git a/test/Parse/availability_query.swift b/test/Parse/availability_query.swift
@@ -42,7 +42,7 @@ if #available(OSX 0) { // expected-warning {{expected version number; this is an
 if #available(OSX 0.0) { // expected-warning {{expected version number; this is an error in the Swift 6 language mode}}
 }
 
-if #available(OSX 51 { // expected-error {{expected ')'}} expected-note {{to match this opening '('}} expected-error {{must handle potential future platforms with '*'}} {{21-21=, *}}
+if #available(OSX 51 { // expected-error {{expected ')'}} expected-note {{to match this opening '('}}
 }
 
 if #available(iDishwasherOS 51) { // expected-warning {{unrecognized platform name 'iDishwasherOS'}}
diff --git a/test/attr/attr_availability.swift b/test/attr/attr_availability.swift
diff --git a/test/attr/attr_availability_swift.swift b/test/attr/attr_availability_swift.swift

Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ if #available(OSX 0) { // expected-warning {{expected version number; this is an`
`42`	`42`	`if #available(OSX 0.0) { // expected-warning {{expected version number; this is an error in the Swift 6 language mode}}`
`43`	`43`	`}`
`44`	`44`
`45`		`-if #available(OSX 51 { // expected-error {{expected ')'}} expected-note {{to match this opening '('}} expected-error {{must handle potential future platforms with ''}} {{21-21=, }}`
	`45`	`+if #available(OSX 51 { // expected-error {{expected ')'}} expected-note {{to match this opening '('}}`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`if #available(iDishwasherOS 51) { // expected-warning {{unrecognized platform name 'iDishwasherOS'}}`