Sema: Fix double type checking of lazy initializer expressions

If a lazy var has no declared type, we have to type check the
initializer to get a type before we can build the getter.

Then, the initializer is type checked as part of the getter
again.

Use the new SkipApplyingSolution flag when type checking for
the first time. We still end up doing redundant work, but by
not applying the solution we avoid feeding invalid AST nodes
back into the constraint solver.

This fixes some bad diagnostics and crashes.

Fixes <https://bugs.swift.org/browse/SR-2616> and
<rdar://problem/28313602>.

Author: slavapestov

lib/Sema/CSApply.cpp

@@ -7581,12 +7581,11 @@ Expr *ConstraintSystem::applySolutionShallow(const Solution &solution,
 Expr *Solution::coerceToType(Expr *expr, Type toType,
                              ConstraintLocator *locator,
                              bool ignoreTopLevelInjection,
-                             bool skipClosures,
                              Optional<Pattern*> typeFromPattern) const {
   auto &cs = getConstraintSystem();
   ExprRewriter rewriter(cs, *this,
                         /*suppressDiagnostics=*/false,
-                        /*skipClosures=*/skipClosures);
+                        /*skipClosures=*/false);
   Expr *result = rewriter.coerceToType(expr, toType, locator, typeFromPattern);
   if (!result)
     return nullptr;

lib/Sema/ConstraintSystem.h

@@ -575,17 +575,13 @@ class Solution {
   /// on a suspicious top-level optional injection (because the caller already
   /// diagnosed it).
   ///
-  /// \param skipClosures Whether to skip bodies of non-single expression
-  /// closures.
-  ///
   /// \param typeFromPattern Optionally, the caller can specify the pattern
   /// from where the toType is derived, so that we can deliver better fixit.
   ///
   /// \returns the coerced expression, which will have type \c ToType.
   Expr *coerceToType(Expr *expr, Type toType,
                      ConstraintLocator *locator,
                      bool ignoreTopLevelInjection = false,
-                     bool skipClosures = false,
                      Optional<Pattern*> typeFromPattern = None) const;
 
   /// \brief Convert the given expression to a logic value.

lib/Sema/TypeCheckConstraints.cpp

@@ -2125,13 +2125,12 @@ bool TypeChecker::typeCheckExpressionShallow(Expr *&expr, DeclContext *dc) {
 }
 
 bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
-                                   DeclContext *DC, bool skipClosures) {
+                                   DeclContext *DC, bool skipApplyingSolution) {
 
   /// Type checking listener for pattern binding initializers.
   class BindingListener : public ExprTypeCheckListener {
     Pattern *&pattern;
     Expr *&initializer;
-    bool skipClosures;
 
     /// The locator we're using.
     ConstraintLocator *Locator;
@@ -2140,10 +2139,9 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
     Type InitType;
 
   public:
-    explicit BindingListener(Pattern *&pattern, Expr *&initializer,
-                             bool skipClosures)
+    explicit BindingListener(Pattern *&pattern, Expr *&initializer)
       : pattern(pattern), initializer(initializer),
-        skipClosures(skipClosures), Locator(nullptr) { }
+        Locator(nullptr) { }
 
     Type getInitType() const { return InitType; }
 
@@ -2177,8 +2175,7 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
       // Convert the initializer to the type of the pattern.
       // ignoreTopLevelInjection = Binding->isConditional()
       expr = solution.coerceToType(expr, InitType, Locator,
-                                   false /* ignoreTopLevelInjection */,
-                                   skipClosures);
+                                   false /* ignoreTopLevelInjection */);
       if (!expr) {
         return nullptr;
       }
@@ -2191,7 +2188,7 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
   };
 
   assert(initializer && "type-checking an uninitialized binding?");
-  BindingListener listener(pattern, initializer, skipClosures);
+  BindingListener listener(pattern, initializer);
 
   TypeLoc contextualType;
   auto contextualPurpose = CTP_Unused;
@@ -2213,8 +2210,8 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
 
   // Type-check the initializer.
   TypeCheckExprOptions flags = TypeCheckExprFlags::ConvertTypeIsOnlyAHint;
-  if (skipClosures)
-    flags |= TypeCheckExprFlags::SkipMultiStmtClosures;
+  if (skipApplyingSolution)
+    flags |= TypeCheckExprFlags::SkipApplyingSolution;
 
   bool hadError = typeCheckExpression(initializer, DC, contextualType,
                                      contextualPurpose,
@@ -2262,7 +2259,7 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
 
 bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD,
                                           unsigned patternNumber,
-                                          bool skipClosures) {
+                                          bool skipApplyingSolution) {
 
   Pattern *pattern = PBD->getPattern(patternNumber);
   Expr *init = PBD->getInit(patternNumber);
@@ -2282,11 +2279,10 @@ bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD,
       DC = initContext;
   }
 
-  bool hadError = typeCheckBinding(pattern, init, DC, skipClosures);
+  bool hadError = typeCheckBinding(pattern, init, DC, skipApplyingSolution);
   PBD->setPattern(patternNumber, pattern, initContext);
   PBD->setInit(patternNumber, init);
 
-
   // If we entered an initializer context, contextualize any
   // auto-closures we might have created.
   if (initContext) {
@@ -2620,7 +2616,8 @@ bool TypeChecker::typeCheckStmtCondition(StmtCondition &cond, DeclContext *dc,
     // If the pattern didn't get a type, it's because we ran into some
     // unknown types along the way. We'll need to check the initializer.
     auto init = elt.getInitializer();
-    hadError |= typeCheckBinding(pattern, init, dc, /*skipClosures*/false);
+    hadError |= typeCheckBinding(pattern, init, dc,
+                                 /*skipApplyingSolution*/false);
     elt.setPattern(pattern);
     elt.setInitializer(init);
     hadAnyFalsable |= pattern->isRefutablePattern();
@@ -2937,7 +2934,6 @@ bool TypeChecker::convertToType(Expr *&expr, Type type, DeclContext *dc,
   Expr *result = solution.coerceToType(expr, type,
                                        cs.getConstraintLocator(expr),
                                        /*ignoreTopLevelInjection*/false,
-                                       /*skipClosures*/false,
                                        typeFromPattern);
   if (!result) {
     return true;

lib/Sema/TypeCheckDecl.cpp

@@ -1198,11 +1198,11 @@ static void validatePatternBindingEntry(TypeChecker &tc,
   // If the pattern didn't get a type or if it contains an unbound generic type,
   // we'll need to check the initializer.
   if (!pattern->hasType() || pattern->getType()->hasUnboundGenericType()) {
-    bool skipClosures = false;
+    bool skipApplyingSolution = false;
     if (auto var = binding->getSingleVar())
-      skipClosures = var->getAttrs().hasAttribute<LazyAttr>();
+      skipApplyingSolution = var->getAttrs().hasAttribute<LazyAttr>();
 
-    if (tc.typeCheckPatternBinding(binding, entryNumber, skipClosures))
+    if (tc.typeCheckPatternBinding(binding, entryNumber, skipApplyingSolution))
       return;
   }
 
@@ -4047,7 +4047,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     if (!IsFirstPass) {
       for (unsigned i = 0, e = PBD->getNumPatternEntries(); i != e; ++i) {
         if (!PBD->isInitializerChecked(i) && PBD->getInit(i))
-          TC.typeCheckPatternBinding(PBD, i, /*skipClosures*/false);
+          TC.typeCheckPatternBinding(PBD, i, /*skipApplyingSolution*/false);
       }
     }
 
@@ -4078,7 +4078,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
             // If we got a default initializer, install it and re-type-check it
             // to make sure it is properly coerced to the pattern type.
             PBD->setInit(i, defaultInit);
-            TC.typeCheckPatternBinding(PBD, i, /*skipClosures*/false);
+            TC.typeCheckPatternBinding(PBD, i, /*skipApplyingSolution*/false);
           }
         }
       }

lib/Sema/TypeChecker.h

@@ -1677,9 +1677,9 @@ class TypeChecker final : public LazyResolver {
 
   /// Type-check an initialized variable pattern declaration.
   bool typeCheckBinding(Pattern *&P, Expr *&Init, DeclContext *DC,
-                        bool skipClosures);
+                        bool skipApplyingSolution);
   bool typeCheckPatternBinding(PatternBindingDecl *PBD, unsigned patternNumber,
-                               bool skipClosures);
+                               bool skipApplyingSolution);
 
   /// Type-check a for-each loop's pattern binding and sequence together.
   bool typeCheckForEachBinding(DeclContext *dc, ForEachStmt *stmt);

test/decl/var/lazy_properties.swift

@@ -27,21 +27,20 @@ class TestClass {
 
   lazy var (e, f) = (1,2)  // expected-error {{'lazy' cannot destructure an initializer}} {{3-8=}}
 
-  lazy var g : Int = { 0 }()   // single-expr closure
+  lazy var g = { 0 }()   // single-expr closure
 
-  lazy var h : Int = { return 0 }()+1  // multi-stmt closure
+  lazy var h : Int = { 0 }()   // single-expr closure
 
-  lazy var i : Int = 42 {  // expected-error {{lazy properties may not have observers}} {{3-8=}}
-    didSet {
-    }
-  }
+  lazy var i = { () -> Int in return 0 }()+1  // multi-stmt closure
+
+  lazy var j : Int = { return 0 }()+1  // multi-stmt closure
+
+  lazy var k : Int = { () -> Int in return 0 }()+1  // multi-stmt closure
 
-  // Lazy values can have observers, be NSCopying, etc.
-/*  lazy var d : Int = 42 {
+  lazy var l : Int = 42 {  // expected-error {{lazy properties may not have observers}} {{3-8=}}
     didSet {
-      print("set me")
     }
-  }*/
+  }
 
   init() {
     lazy var localvar = 42  // expected-error {{lazy is only valid for members of a struct or class}} {{5-10=}}
@@ -91,3 +90,32 @@ class WeShouldNotReTypeCheckStatements {
     _ = ()
   }
 }
+
+protocol MyProtocol {
+  func f() -> Int
+}
+
+struct MyStruct : MyProtocol {
+  func f() -> Int { return 0 }
+}
+
+struct Outer {
+  static let p: MyProtocol = MyStruct()
+
+  struct Inner {
+    lazy var x = p.f()
+
+    lazy var y = {_ = 3}()
+    // expected-warning@-1 {{variable 'y' inferred to have type '()', which may be unexpected}}
+    // expected-note@-2 {{add an explicit type annotation to silence this warning}}
+  }
+}
+
+// https://bugs.swift.org/browse/SR-2616
+struct Construction {
+  init(x: Int, y: Int? = 42) { }
+}
+
+class Constructor {
+  lazy var myQ = Construction(x: 3)
+}

validation-test/compiler_crashers/28508-unreachable-executed-at-swift-lib-sema-csgen-cpp-2656.swift renamed to validation-test/compiler_crashers_fixed/28508-unreachable-executed-at-swift-lib-sema-csgen-cpp-2656.swift

@@ -5,5 +5,5 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 protocol A{{}func a{}struct A{lazy var f=a

validation-test/compiler_crashers/28509-allowoverwrite-e-haslvalueaccesskind-l-value-access-kind-has-already-been-set.swift renamed to validation-test/compiler_crashers_fixed/28509-allowoverwrite-e-haslvalueaccesskind-l-value-access-kind-has-already-been-set.swift

@@ -5,6 +5,5 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// RUN: not --crash %target-swift-frontend %s -emit-ir
-// REQUIRES: asserts
+// RUN: not %target-swift-frontend %s -emit-ir
 class d{lazy var f={_={