Merge pull request #84994 from beccadax/mod-squad-arg-list

[SE-0491] Fix malformed arg label error with module selector

Author: beccadax

include/swift/Parse/Parser.h

@@ -1817,8 +1817,11 @@ class Parser {
   /// \param loc The location of the label (empty if it doesn't exist)
   /// \param isAttr True if this is an argument label for an attribute (allows, but deprecates, use of
   ///               \c '=' instead of \c ':').
+  /// \param splittingColonColon True if \c :: tokens should be treated as two
+  ///                           adjacent colons.
   void parseOptionalArgumentLabel(Identifier &name, SourceLoc &loc,
-                                  bool isAttr = false);
+                                  bool isAttr = false,
+                                  bool splittingColonColon = false);
 
   /// If a \c module-selector is present, returns a true value (specifically,
   /// 1 or 2 depending on how many tokens should be consumed to skip it).

lib/Parse/ParseExpr.cpp

@@ -2154,29 +2154,15 @@ ParserResult<Expr> Parser::parseExprStringLiteral() {
                                       AppendingExpr));
 }
 
-/// Equivalent to \c Tok.is(tok::colon), but pretends that \c tok::colon_colon
-/// doesn't exist if \c EnableExperimentalModuleSelector is disabled.
-static bool isColon(Parser &P, Token Tok, tok altColon = tok::NUM_TOKENS) {
-  // FIXME: Introducing tok::colon_colon broke diag::empty_arg_label_underscore.
-  // We only care about tok::colon_colon when module selectors are turned on, so
-  // when they are turned off, this function works around the bug by treating
-  // tok::colon_colon as a synonym for tok::colon. However, the bug still exists
-  // when Feature::ModuleSelector is enabled. We will need to address this
-  // before the feature can be released.
-
-  if (P.Context.LangOpts.hasFeature(Feature::ModuleSelector))
-    return Tok.isAny(tok::colon, altColon);
-
-  return Tok.isAny(tok::colon, tok::colon_colon, altColon);
-}
-
 void Parser::parseOptionalArgumentLabel(Identifier &name, SourceLoc &loc,
-                                        bool isAttr) {
+                                        bool isAttr, bool splittingColonColon) {
+  auto colonColon = splittingColonColon ? tok::colon_colon : tok::NUM_TOKENS;
   /// A token that has the same meaning as colon, but is deprecated, if one exists for this call.
   auto altColon = isAttr ? tok::equal : tok::NUM_TOKENS;
 
   // Check to see if there is an argument label.
-  if (Tok.canBeArgumentLabel() && isColon(*this, peekToken(), altColon)) {
+  if (Tok.canBeArgumentLabel() && peekToken().isAny(tok::colon, colonColon,
+                                                    altColon)) {
     auto text = Tok.getText();
 
     // If this was an escaped identifier that need not have been escaped, say
@@ -2195,7 +2181,7 @@ void Parser::parseOptionalArgumentLabel(Identifier &name, SourceLoc &loc,
     }
 
     loc = consumeArgumentLabel(name, /*diagnoseDollarPrefix=*/false);
-  } else if (isColon(*this, Tok, altColon)) {
+  } else if (Tok.isAny(tok::colon, colonColon, altColon)) {
     // Found only the colon.
     diagnose(Tok, diag::expected_label_before_colon)
       .fixItInsert(Tok.getLoc(), "<#label#>");
@@ -2205,17 +2191,13 @@ void Parser::parseOptionalArgumentLabel(Identifier &name, SourceLoc &loc,
   }
 
   // If we get here, we ought to be on the colon.
-  ASSERT(Tok.isAny(tok::colon, tok::colon_colon, altColon));
-
-  if (Tok.is(tok::colon_colon)) {
-    consumeIfColonSplittingDoubles();
+  if (consumeIfColonSplittingDoubles())
     return;
-  }
 
-  if (Tok.is(altColon))
-    diagnose(Tok, diag::replace_equal_with_colon_for_value)
-      .fixItReplace(Tok.getLoc(), ": ");
+  ASSERT(Tok.is(altColon));
 
+  diagnose(Tok, diag::replace_equal_with_colon_for_value)
+    .fixItReplace(Tok.getLoc(), ": ");
   consumeToken();
 }
 
@@ -2240,7 +2222,10 @@ static bool tryParseArgLabelList(Parser &P, Parser::DeclNameOptions flags,
       flags.contains(Parser::DeclNameFlag::AllowZeroArgCompoundNames) &&
       next.is(tok::r_paren);
   // An argument label.
-  bool nextIsArgLabel = next.canBeArgumentLabel() || isColon(P, next);
+  // We allow `tok::colon_colon` so that e.g. `fn(::)` gets diagnosed as a
+  // malformed version of `fn(_:_:)`.
+  bool nextIsArgLabel = next.canBeArgumentLabel()
+                          || next.isAny(tok::colon, tok::colon_colon);
   // An editor placeholder.
   bool nextIsPlaceholder = next.isEditorPlaceholder();
 
@@ -2262,7 +2247,8 @@ static bool tryParseArgLabelList(Parser &P, Parser::DeclNameOptions flags,
 
     Identifier argName;
     SourceLoc argLoc;
-    P.parseOptionalArgumentLabel(argName, argLoc);
+    P.parseOptionalArgumentLabel(argName, argLoc, /*isAttr=*/false,
+                                 /*splittingColonColon=*/true);
     if (argLoc.isValid()) {
       argumentLabels.push_back(argName);
       argumentLabelLocs.push_back(argLoc);

test/expr/primary/unqualified_name.swift

@@ -1,5 +1,8 @@
 // RUN: %target-typecheck-verify-swift -swift-version 4
 
+// RUN: %target-typecheck-verify-swift -swift-version 4 -enable-experimental-feature ModuleSelector
+// REQUIRES: swift_feature_ModuleSelector
+
 func f0(_ x: Int, y: Int, z: Int) { }
 func f1(_ x: Int, while: Int) { }
 func f2(_ x: Int, `let` _: Int) { }