Merge pull request swiftlang#31760 from CodaFi/patternak

Remove The Parser Hack For If-Let

Author: CodaFi

include/swift/Parse/Parser.h

@@ -1359,8 +1359,7 @@ class Parser {
   ParserResult<Pattern> parsePatternTuple();
 
   ParserResult<Pattern>
-  parseOptionalPatternTypeAnnotation(ParserResult<Pattern> P,
-                                     bool isOptional);
+  parseOptionalPatternTypeAnnotation(ParserResult<Pattern> P);
   ParserResult<Pattern> parseMatchingPattern(bool isExprBasic);
   ParserResult<Pattern> parseMatchingPatternAsLetOrVar(bool isLet,
                                                        SourceLoc VarLoc,

lib/AST/ASTPrinter.cpp

@@ -2613,18 +2613,10 @@ void PrintAST::visitVarDecl(VarDecl *decl) {
   Printer << ": ";
   TypeLoc tyLoc;
   if (auto *repr = decl->getTypeReprOrParentPatternTypeRepr()) {
-    // Workaround for if-let statements. The parser creates a `OptionalTypeRepr`
-    // even though the user-written declared type for the if-let variable
-    // is non-optional. Get the non-optional type so we can print it correctly.
-    if (auto *optRepr = dyn_cast<OptionalTypeRepr>(repr)) {
-      if (type && !isa<OptionalType>(type.getPointer())) {
-        repr = optRepr->getBase();
-      }
-    }
     tyLoc = TypeLoc(repr, type);
-  } else
+  } else {
     tyLoc = TypeLoc::withoutLoc(type);
-
+  }
   Printer.printDeclResultTypePre(decl, tyLoc);
 
   // HACK: When printing result types for vars with opaque result types,

lib/Parse/ParsePattern.cpp

@@ -1124,8 +1124,7 @@ ParserResult<Pattern> Parser::parsePatternTuple() {
 ///  pattern-type-annotation ::= (':' type)?
 ///
 ParserResult<Pattern> Parser::
-parseOptionalPatternTypeAnnotation(ParserResult<Pattern> result,
-                                   bool isOptional) {
+parseOptionalPatternTypeAnnotation(ParserResult<Pattern> result) {
   if (!Tok.is(tok::colon))
     return result;
 
@@ -1152,15 +1151,6 @@ parseOptionalPatternTypeAnnotation(ParserResult<Pattern> result,
   if (!repr)
     repr = new (Context) ErrorTypeRepr(PreviousLoc);
 
-  // In an if-let, the actual type of the expression is Optional of whatever
-  // was written.
-  // FIXME: This is not good, `TypeRepr`s are supposed to represent what the
-  // user actually wrote in source (that's why they don't have any `isImplicit`
-  // bit). This synthesized `OptionalTypeRepr` leads to workarounds in other
-  // parts where we want to reason about the types as perceived by the user.
-  if (isOptional)
-    repr = new (Context) OptionalTypeRepr(repr, SourceLoc());
-
   return makeParserResult(status, new (Context) TypedPattern(P, repr));
 }
 

lib/Parse/ParseStmt.cpp

@@ -1502,8 +1502,7 @@ Parser::parseStmtConditionElement(SmallVectorImpl<StmtConditionElement> &result,
       P->setImplicit();
   }
 
-  ThePattern = parseOptionalPatternTypeAnnotation(ThePattern,
-                                                  BindingKindStr != "case");
+  ThePattern = parseOptionalPatternTypeAnnotation(ThePattern);
   if (ThePattern.hasCodeCompletion())
     Status.setHasCodeCompletion();
 
@@ -2123,7 +2122,7 @@ ParserResult<Stmt> Parser::parseStmtForEach(LabeledStmtInfo LabelInfo) {
     llvm::SaveAndRestore<decltype(InVarOrLetPattern)>
       T(InVarOrLetPattern, Parser::IVOLP_InMatchingPattern);
     pattern = parseMatchingPattern(/*isExprBasic*/true);
-    pattern = parseOptionalPatternTypeAnnotation(pattern, /*isOptional*/false);
+    pattern = parseOptionalPatternTypeAnnotation(pattern);
   } else if (!IsCStyleFor || Tok.is(tok::kw_var)) {
     // Change the parser state to know that the pattern we're about to parse is
     // implicitly mutable.  Bound variables can be changed to mutable explicitly

lib/Sema/ConstraintSystem.cpp

@@ -4284,7 +4284,7 @@ SolutionApplicationTarget SolutionApplicationTarget::forInitialization(
     bool bindPatternVarsOneWay) {
   // Determine the contextual type for the initialization.
   TypeLoc contextualType;
-  if (!isa<OptionalSomePattern>(pattern) &&
+  if (!(isa<OptionalSomePattern>(pattern) && !pattern->isImplicit()) &&
       patternType && !patternType->isHole()) {
     contextualType = TypeLoc::withoutLoc(patternType);
 

lib/Sema/ConstraintSystem.h

@@ -1444,7 +1444,8 @@ class SolutionApplicationTarget {
   bool isOptionalSomePatternInit() const {
     return kind == Kind::expression &&
         expression.contextualPurpose == CTP_Initialization &&
-        isa<OptionalSomePattern>(expression.pattern);
+        isa<OptionalSomePattern>(expression.pattern) &&
+        !expression.pattern->isImplicit();
   }
 
   /// Whether to bind the types of any variables within the pattern via

lib/Sema/MiscDiagnostics.cpp

@@ -3752,10 +3752,9 @@ checkImplicitPromotionsInCondition(const StmtConditionElement &cond,
     // checking for a type, which forced it to be promoted to a double optional
     // type.
     if (auto ooType = subExpr->getType()->getOptionalObjectType()) {
-      if (auto TP = dyn_cast<TypedPattern>(p))
+      if (auto OSP = dyn_cast<OptionalSomePattern>(p)) {
         // Check for 'if let' to produce a tuned diagnostic.
-        if (isa<OptionalSomePattern>(TP->getSubPattern()) &&
-            TP->getSubPattern()->isImplicit()) {
+        if (auto *TP = dyn_cast<TypedPattern>(OSP->getSubPattern())) {
           ctx.Diags.diagnose(cond.getIntroducerLoc(),
                              diag::optional_check_promotion,
                              subExpr->getType())
@@ -3764,6 +3763,7 @@ checkImplicitPromotionsInCondition(const StmtConditionElement &cond,
                           ooType->getString());
           return;
         }
+      }
       ctx.Diags.diagnose(cond.getIntroducerLoc(),
                          diag::optional_pattern_match_promotion,
                          subExpr->getType(), cond.getInitializer()->getType())

lib/Sema/TypeCheckPattern.cpp

@@ -655,12 +655,8 @@ Pattern *TypeChecker::resolvePattern(Pattern *P, DeclContext *DC,
 
     // "if let" implicitly looks inside of an optional, so wrap it in an
     // OptionalSome pattern.
-    InnerP = new (Context) OptionalSomePattern(InnerP, InnerP->getEndLoc());
-    InnerP->setImplicit();
-    if (auto *TP = dyn_cast<TypedPattern>(P))
-      TP->setSubPattern(InnerP);
-    else
-      P = InnerP;
+    P = new (Context) OptionalSomePattern(P, P->getEndLoc());
+    P->setImplicit();
   }
 
   return P;
@@ -811,9 +807,24 @@ Type PatternTypeRequest::evaluate(Evaluator &evaluator,
   //
   // Refutable patterns occur when checking the PatternBindingDecls in if/let,
   // while/let, and let/else conditions.
+  case PatternKind::OptionalSome: {
+    // Annotated if-let patterns are rewritten by TypeChecker::resolvePattern
+    // to have an enclosing implicit (...)? pattern. If we can resolve the inner
+    // typed pattern, the resulting pattern must have optional type.
+    auto somePat = cast<OptionalSomePattern>(P);
+    if (somePat->isImplicit() && isa<TypedPattern>(somePat->getSubPattern())) {
+      auto resolution = TypeResolution::forContextual(dc, options);
+      TypedPattern *TP = cast<TypedPattern>(somePat->getSubPattern());
+      auto type = validateTypedPattern(resolution, TP, options);
+      if (type && !type->hasError()) {
+        return OptionalType::get(type);
+      }
+    }
+    LLVM_FALLTHROUGH;
+  }
+
   case PatternKind::Is:
   case PatternKind::EnumElement:
-  case PatternKind::OptionalSome:
   case PatternKind::Bool:
   case PatternKind::Expr:
     // In a let/else, these always require an initial value to match against.

test/stmt/statements.swift

@@ -556,6 +556,7 @@ func testThrowNil() throws {
 // condition may have contained a SequenceExpr.
 func r23684220(_ b: Any) {
   if let _ = b ?? b {} // expected-warning {{left side of nil coalescing operator '??' has non-optional type 'Any', so the right side is never used}}
+  // expected-error@-1 {{initializer for conditional binding must have Optional type, not 'Any'}}
 }