Parse: Refactor platform version tuple list parsing for @_backDeploy into a separate function and improve diagnostics.

Author: tshortli

include/swift/AST/DiagnosticsParse.def

@@ -1503,6 +1503,8 @@ WARNING(attr_availability_invalid_duplicate,none,
         "'%0' argument has already been specified", (StringRef))
 WARNING(attr_availability_unknown_platform,none,
       "unknown platform '%0' for attribute '%1'", (StringRef, StringRef))
+ERROR(attr_availability_expected_platform,none,
+      "expected platform in '%0' attribute", (StringRef))
 ERROR(attr_availability_invalid_renamed,none,
       "'renamed' argument of '%0' attribute must be an operator, identifier, "
       "or full function name, optionally prefixed by a type name", (StringRef))

include/swift/Parse/Parser.h

@@ -1846,6 +1846,16 @@ class Parser {
                     bool &DiscardAttribute, SourceRange &attrRange,
                     SourceLoc AtLoc, SourceLoc Loc,
                     llvm::function_ref<void(AvailableAttr *)> addAttribute);
+
+  using PlatformAndVersion = std::pair<PlatformKind, llvm::VersionTuple>;
+
+  /// Parse a platform and version tuple (e.g. "macOS 12.0") and append it to the
+  /// given vector. Wildcards ('*') parse successfully but are ignored. Assumes
+  /// that the tuples are part of a comma separated list ending with a trailing
+  /// ')'.
+  ParserStatus parsePlatformVersionInList(StringRef AttrName,
+      llvm::SmallVector<PlatformAndVersion, 4> &PlatformAndVersions);
+
   //===--------------------------------------------------------------------===//
   // Code completion second pass.
 

lib/Parse/ParseDecl.cpp

@@ -1738,6 +1738,64 @@ void Parser::parseAllAvailabilityMacroArguments() {
   AvailabilityMacrosComputed = true;
 }
 
+ParserStatus Parser::parsePlatformVersionInList(StringRef AttrName,
+    llvm::SmallVector<PlatformAndVersion, 4> &PlatformAndVersions) {
+  // FIXME(backDeploy): Parse availability macros (e.g. SwiftStdlib: 5.1)
+  SyntaxParsingContext argumentContext(SyntaxContext,
+      SyntaxKind::AvailabilityVersionRestriction);
+
+  // Expect a possible platform name (e.g. 'macOS' or '*').
+  if (!Tok.isAny(tok::identifier, tok::oper_binary_spaced)) {
+    diagnose(Tok, diag::attr_availability_expected_platform, AttrName);
+    return makeParserError();
+  }
+
+  // Parse the platform name.
+  auto MaybePlatform = platformFromString(Tok.getText());
+  SourceLoc PlatformLoc = Tok.getLoc();
+  if (!MaybePlatform.hasValue()) {
+    diagnose(PlatformLoc, diag::attr_availability_unknown_platform,
+             Tok.getText(), AttrName);
+    return makeParserError();
+  }
+  consumeToken();
+  PlatformKind Platform = *MaybePlatform;
+
+  // Wildcards ('*') aren't supported in this kind of list. If this list
+  // entry is just a wildcard, skip it. Wildcards with a version are
+  // diagnosed below.
+  if (Platform == PlatformKind::none && Tok.isAny(tok::comma, tok::r_paren)) {
+    diagnose(PlatformLoc, diag::attr_availability_wildcard_ignored,
+             AttrName);
+    return makeParserSuccess();
+  }
+
+  // Parse version number.
+  llvm::VersionTuple VerTuple;
+  SourceRange VersionRange;
+  if (parseVersionTuple(VerTuple, VersionRange,
+      Diagnostic(diag::attr_availability_expected_version, AttrName))) {
+    return makeParserError();
+  }
+
+  // Diagnose specification of patch versions (e.g. '13.0.1').
+  if (VerTuple.getSubminor().hasValue() ||
+      VerTuple.getBuild().hasValue()) {
+    diagnose(VersionRange.Start,
+             diag::attr_availability_platform_version_major_minor_only,
+             AttrName);
+  }
+
+  // Wildcards ('*') aren't supported in this kind of list.
+  if (Platform == PlatformKind::none) {
+    diagnose(PlatformLoc, diag::attr_availability_wildcard_ignored,
+             AttrName);
+  } else {
+    PlatformAndVersions.emplace_back(Platform, VerTuple);
+  }
+  return makeParserSuccess();
+}
+
 /// Processes a parsed option name by attempting to match it to a list of
 /// alternative name/value pairs provided by a chain of \c when() calls, ending
 /// in either \c whenOmitted() if omitting the option is allowed, or
@@ -2862,65 +2920,27 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc,
                DeclAttribute::isDeclModifier(DK));
       return false;
     }
-    SourceLoc RightLoc;
+
     bool SuppressLaterDiags = false;
-    llvm::SmallVector<std::pair<PlatformKind, llvm::VersionTuple>, 4>
-      PlatformAndVersions;
-    
+    SourceLoc RightLoc;
+    llvm::SmallVector<PlatformAndVersion, 4> PlatformAndVersions;
     StringRef AttrName = "@_backDeploy";
-    if (parseList(tok::r_paren, LeftLoc, RightLoc, false,
-                  diag::attr_back_deploy_missing_rparen,
-                  SyntaxKind::Unknown, [&]() -> ParserStatus {
-      // FIXME(backDeploy): Parse availability macros (e.g. SwiftStdlib: 5.1)
-      
-      // Parse a platform and version tuple (e.g. 'macOS 13.13').
-      if ((Tok.is(tok::identifier) || Tok.is(tok::oper_binary_spaced)) &&
-          (peekToken().is(tok::floating_literal) ||
-           peekToken().is(tok::integer_literal))) {
-        PlatformKind Platform;
-        // Parse platform name.
-        auto Plat = platformFromString(Tok.getText());
-        if (!Plat.hasValue()) {
-          diagnose(Tok.getLoc(), diag::attr_availability_unknown_platform,
-                   Tok.getText(), AttrName);
-          SuppressLaterDiags = true;
-          return makeParserError();
-        } else {
-          consumeToken();
-          Platform = *Plat;
-        }
-        // Parse version number
-        llvm::VersionTuple VerTuple;
-        SourceRange VersionRange;
-        if (parseVersionTuple(VerTuple, VersionRange,
-              Diagnostic(diag::attr_availability_expected_version, AttrName))) {
-          SuppressLaterDiags = true;
-          return makeParserError();
-        } else {
-          if (VerTuple.getSubminor().hasValue() ||
-              VerTuple.getBuild().hasValue()) {
-            diagnose(Tok.getLoc(),
-                     diag::attr_availability_platform_version_major_minor_only,
-                     AttrName);
-          }
-          // * as platform name isn't supported.
-          if (Platform == PlatformKind::none) {
-            diagnose(AtLoc, diag::attr_availability_wildcard_ignored, AttrName);
-          } else {
-            PlatformAndVersions.emplace_back(Platform, VerTuple);
-          }
-          return makeParserSuccess();
-        }
-      }
-      diagnose(AtLoc, diag::attr_availability_need_platform_version, AttrName);
-      SuppressLaterDiags = true;
-      return makeParserError();
-    }).isErrorOrHasCompletion() || SuppressLaterDiags) {
+    ParserStatus Status = parseList(tok::r_paren, LeftLoc, RightLoc, false,
+                                    diag::attr_back_deploy_missing_rparen,
+                                    SyntaxKind::Unknown, [&]() -> ParserStatus {
+      ParserStatus ListItemStatus =
+          parsePlatformVersionInList(AttrName, PlatformAndVersions);
+      if (ListItemStatus.isErrorOrHasCompletion())
+        SuppressLaterDiags = true;
+      return ListItemStatus;
+    });
+
+    if (Status.isErrorOrHasCompletion() || SuppressLaterDiags) {
       return false;
     }
 
     if (PlatformAndVersions.empty()) {
-      diagnose(AtLoc, diag::attr_availability_need_platform_version, AttrName);
+      diagnose(Loc, diag::attr_availability_need_platform_version, AttrName);
       return false;
     }
 

test/attr/backDeploy.swift

@@ -66,10 +66,25 @@ public var cannotBackDeployTopLevelVar = 79
 @_backDeploy(macOS 11.0, unknownOS 1.0) // expected-warning {{unknown platform 'unknownOS' for attribute '@_backDeploy'}}
 public func unknownOSFunc() {}
 
-// FIXME(backDeploy): The error here should be specific about the missing version for iOS
 @available(macOS 12.0, *)
-@_backDeploy(macOS 11.0, iOS) // expected-error {{expected at least one platform version in in '@_backDeploy' attribute}}
-public func missingVersionForPlatformFunc() {}
+@_backDeploy(@) // expected-error {{expected platform in '@_backDeploy' attribute}}
+public func badPlatformFunc1() {}
+
+@available(macOS 12.0, *)
+@_backDeploy(@ 12.0) // expected-error {{expected platform in '@_backDeploy' attribute}}
+public func badPlatformFunc2() {}
+
+@available(macOS 12.0, *)
+@_backDeploy(macOS) // expected-error {{expected version number in '@_backDeploy' attribute}}
+public func missingVersionFunc1() {}
+
+@available(macOS 12.0, *)
+@_backDeploy(macOS 11.0, iOS) // expected-error {{expected version number in '@_backDeploy' attribute}}
+public func missingVersionFunc2() {}
+
+@available(macOS 12.0, *)
+@_backDeploy(macOS, iOS) // expected-error 2{{expected version number in '@_backDeploy' attribute}}
+public func missingVersionFunc3() {}
 
 @available(macOS 12.0, *)
 @_backDeploy(macOS 11.0, iOS 14.0,) // expected-error {{unexpected ',' separator}}
@@ -81,12 +96,15 @@ public func patchVersionFunc() {}
 
 @available(macOS 12.0, *)
 @_backDeploy(macOS 11.0, * 9.0) // expected-warning {{* as platform name has no effect in '@_backDeploy' attribute}}
-public func wildcardPlatformNameFunc() {}
+public func wildcardWithVersionFunc() {}
+
+@available(macOS 12.0, *)
+@_backDeploy(macOS 11.0, *) // expected-warning {{* as platform name has no effect in '@_backDeploy' attribute}}
+public func trailingWildcardFunc() {}
 
-// FIXME(backDeploy): The error here should be about * being ignored
 @available(macOS 12.0, *)
-@_backDeploy(macOS 11.0, *) // expected-error {{expected at least one platform version in in '@_backDeploy' attribute}}
-public func trailingWildcardPlatformFunc() {}
+@_backDeploy(macOS 11.0, *, iOS 14.0) // expected-warning {{* as platform name has no effect in '@_backDeploy' attribute}}
+public func embeddedWildcardFunc() {}
 
 // FIXME(backDeploy): Expect error for duplicate platforms in same attribute
 @available(macOS 12.0, *)