Remove The Parser Hack For If-Let

The parser used to rewrite

if let x: T

into

if let x: T?

This transformation is correct at face value, but relied on being able
to construct TypeReprs with bogus source locations. Instead of having
the parser kick semantic analysis into shape, let's perform this
reinterpretation when we resolve if-let patterns in statement
conditions.

Author: CodaFi

Diff for: 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,

Diff for: 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));
 }
 

Diff for: 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

Diff for: lib/Sema/ConstraintSystem.cpp

@@ -4278,7 +4278,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);
 

Diff for: lib/Sema/ConstraintSystem.h

@@ -1434,7 +1434,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

Diff for: 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())

Diff for: 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.

Diff for: 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'}}
 }