Merge pull request #27835 from apple/revert-27820-valid-the-impaler

Revert "Remove The Last Vestiges of `isInvalid` from Parse"

Author: shahmishal

include/swift/AST/DiagnosticsParse.def

@@ -870,6 +870,8 @@ ERROR(expected_parameter_colon,PointsToFirstBadToken,
       "expected ':' following argument label and parameter name", ())
 ERROR(expected_assignment_instead_of_comparison_operator,none,
       "expected '=' instead of '==' to assign default value for parameter", ())
+ERROR(missing_parameter_type,PointsToFirstBadToken,
+      "parameter requires an explicit type", ())
 ERROR(multiple_parameter_ellipsis,none,
       "only a single variadic parameter '...' is permitted", ())
 ERROR(parameter_vararg_default,none,

lib/Parse/ParseDecl.cpp

@@ -4933,6 +4933,12 @@ void Parser::ParsedAccessors::record(Parser &P, AbstractStorageDecl *storage,
   storage->setAccessors(LBLoc, Accessors, RBLoc);
 }
 
+static void flagInvalidAccessor(AccessorDecl *func) {
+  if (func) {
+    func->setInvalid();
+  }
+}
+
 static void diagnoseConflictingAccessors(Parser &P, AccessorDecl *first,
                                          AccessorDecl *&second) {
   if (!second) return;
@@ -4943,7 +4949,7 @@ static void diagnoseConflictingAccessors(Parser &P, AccessorDecl *first,
   P.diagnose(first->getLoc(), diag::previous_accessor,
              getAccessorNameForDiagnostic(first, /*article*/ false),
              /*already*/ false);
-  second->setInvalid();
+  flagInvalidAccessor(second);
 }
 
 template <class... DiagArgs>
@@ -4953,11 +4959,11 @@ static void diagnoseAndIgnoreObservers(Parser &P,
                         typename std::enable_if<true, DiagArgs>::type... args) {
   if (auto &accessor = accessors.WillSet) {
     P.diagnose(accessor->getLoc(), diagnostic, /*willSet*/ 0, args...);
-    accessor->setInvalid();
+    flagInvalidAccessor(accessor);
   }
   if (auto &accessor = accessors.DidSet) {
     P.diagnose(accessor->getLoc(), diagnostic, /*didSet*/ 1, args...);
-    accessor->setInvalid();
+    flagInvalidAccessor(accessor);
   }
 }
 
@@ -4969,7 +4975,7 @@ void Parser::ParsedAccessors::classify(Parser &P, AbstractStorageDecl *storage,
   // was invalid.
   if (invalid) {
     for (auto accessor : Accessors) {
-      accessor->setInvalid();
+      flagInvalidAccessor(accessor);
     }
   }
 

lib/Parse/ParseExpr.cpp

@@ -2656,6 +2656,8 @@ parseClosureSignatureIfPresent(SmallVectorImpl<CaptureListEntry> &captureList,
   // to detect any tuple splat or destructuring as early as
   // possible and give a proper fix-it. See SE-0110 for more details.
   auto isTupleDestructuring = [](ParamDecl *param) -> bool {
+    if (!param->isInvalid())
+      return false;
     if (auto *typeRepr = param->getTypeRepr())
       return !param->hasName() && isa<TupleTypeRepr>(typeRepr);
     return false;

lib/Parse/ParsePattern.cpp

@@ -327,9 +327,6 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
         // was invalid.  Remember that.
         if (type.isParseError() && !type.hasCodeCompletion())
           param.isInvalid = true;
-      } else if (paramContext != Parser::ParameterContextKind::Closure) {
-        diagnose(Tok, diag::expected_parameter_colon);
-        param.isInvalid = true;
       }
     } else {
       // Otherwise, we have invalid code.  Check to see if this looks like a
@@ -361,6 +358,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
         // on is most likely argument destructuring, we are going
         // to diagnose that after all of the parameters are parsed.
         if (param.Type) {
+          // Mark current parameter as invalid so it is possible
+          // to diagnose it as destructuring of the closure parameter list.
           param.isInvalid = true;
           if (!isClosure) {
             // Unnamed parameters must be written as "_: Type".
@@ -479,6 +478,12 @@ mapParsedParameters(Parser &parser,
                                      parser.CurDeclContext);
     param->getAttrs() = paramInfo.Attrs;
 
+    auto setInvalid = [&]{
+      if (param->isInvalid())
+        return;
+      param->setInvalid();
+    };
+
     bool parsingEnumElt
       = (paramContext == Parser::ParameterContextKind::EnumElement);
     // If we're not parsing an enum case, lack of a SourceLoc for both
@@ -488,7 +493,7 @@ mapParsedParameters(Parser &parser,
 
     // If we diagnosed this parameter as a parse error, propagate to the decl.
     if (paramInfo.isInvalid)
-      param->setInvalid();
+      setInvalid();
 
     // If a type was provided, create the type for the parameter.
     if (auto type = paramInfo.Type) {
@@ -519,6 +524,13 @@ mapParsedParameters(Parser &parser,
         // or typealias with underlying function type.
         param->setAutoClosure(attrs.has(TypeAttrKind::TAK_autoclosure));
       }
+    } else if (paramContext != Parser::ParameterContextKind::Closure) {
+      // Non-closure parameters require a type.
+      if (!param->isInvalid())
+        parser.diagnose(param->getLoc(), diag::missing_parameter_type);
+
+      param->setSpecifier(ParamSpecifier::Default);
+      setInvalid();
     } else if (paramInfo.SpecifierLoc.isValid()) {
       StringRef specifier;
       switch (paramInfo.SpecifierKind) {

test/Parse/invalid.swift

@@ -76,7 +76,7 @@ protocol Animal<Food> {  // expected-error {{protocols do not allow generic para
 // SR-573 - Crash with invalid parameter declaration
 class Starfish {}
 struct Salmon {}
-func f573(s Starfish,  // expected-error {{expected ':' following argument label and parameter name}}
+func f573(s Starfish,  // expected-error {{parameter requires an explicit type}}
           _ ss: Salmon) -> [Int] {}
 func g573() { f573(Starfish(), Salmon()) }
 
@@ -89,7 +89,7 @@ func SR979b(inout inout b: Int) {} // expected-error {{inout' before a parameter
 // expected-error@-1 {{parameter must not have multiple '__owned', 'inout', or '__shared' specifiers}} {{19-25=}}
 func SR979d(let let a: Int) {} // expected-warning {{'let' in this position is interpreted as an argument label}} {{13-16=`let`}}
 // expected-error @-1 {{expected ',' separator}} {{20-20=,}} 
-// expected-error @-2 {{expected ':' following argument label and parameter name}}
+// expected-error @-2 {{parameter requires an explicit type}}
 // expected-warning @-3 {{extraneous duplicate parameter name; 'let' already has an argument label}} {{13-17=}}
 func SR979e(inout x: inout String) {} // expected-error {{parameter must not have multiple '__owned', 'inout', or '__shared' specifiers}} {{13-18=}}
 func SR979g(inout i: inout Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', or '__shared' specifiers}} {{13-18=}}

test/Sema/immutability.swift

@@ -370,7 +370,7 @@ func testSelectorStyleArguments1(_ x: Int, bar y: Int) {
 func testSelectorStyleArguments2(let x: Int, // expected-warning {{'let' in this position is interpreted as an argument label}}{{34-37=`let`}}
                                  let bar y: Int) { // expected-warning {{'let' in this position is interpreted as an argument label}}{{34-37=`let`}}
 // expected-error @-1 {{expected ',' separator}}
-// expected-error @-2 {{expected ':' following argument label and parameter name}}
+// expected-error @-2 {{parameter requires an explicit type}}
 }
 func testSelectorStyleArguments3(_ x: Int, bar y: Int) {
   ++x  // expected-error {{cannot pass immutable value to mutating operator: 'x' is a 'let' constant}}