Merge pull request #10911 from slavapestov/raii-scrubbers-are-bad-and-should-die-4.0

Fix regression with lazy properties and add another test case [4.0]

Author: slavapestov

lib/Sema/CSDiag.cpp

@@ -8969,6 +8969,8 @@ diagnoseAmbiguousMultiStatementClosure(ClosureExpr *closure) {
       if (hasUnresolvedParams)
         continue;
 
+      CS->TC.preCheckExpression(resultExpr, CS->DC);
+
       // Obtain type of the result expression without applying solutions,
       // because otherwise this might result in leaking of type variables,
       // since we are not resetting result statement and if expression is

lib/Sema/TypeCheckConstraints.cpp

@@ -1600,16 +1600,16 @@ CleanupIllFormedExpressionRAII::~CleanupIllFormedExpressionRAII() {
 
 /// Pre-check the expression, validating any types that occur in the
 /// expression and folding sequence expressions.
-static bool preCheckExpression(TypeChecker &tc, Expr *&expr, DeclContext *dc) {
-  PreCheckExpression preCheck(tc, dc);
+bool TypeChecker::preCheckExpression(Expr *&expr, DeclContext *dc) {
+  PreCheckExpression preCheck(*this, dc);
   // Perform the pre-check.
   if (auto result = expr->walk(preCheck)) {
     expr = result;
     return false;
   }
 
   // Pre-check failed. Clean up and return.
-  CleanupIllFormedExpressionRAII::doIt(expr, tc.Context);
+  CleanupIllFormedExpressionRAII::doIt(expr, Context);
   return true;
 }
 
@@ -1645,11 +1645,6 @@ solveForExpression(Expr *&expr, DeclContext *dc, Type convertType,
                    ExprTypeCheckListener *listener, ConstraintSystem &cs,
                    SmallVectorImpl<Solution> &viable,
                    TypeCheckExprOptions options) {
-  // First, pre-check the expression, validating any types that occur in the
-  // expression and folding sequence expressions.
-  if (preCheckExpression(*this, expr, dc))
-    return true;
-
   // Attempt to solve the constraint system.
   auto solution = cs.solve(expr,
                            convertType,
@@ -1707,6 +1702,7 @@ namespace {
     llvm::SmallVector<Expr*,4> Exprs;
     llvm::SmallVector<TypeLoc*, 4> TypeLocs;
     llvm::SmallVector<Pattern*, 4> Patterns;
+    llvm::SmallVector<VarDecl*, 4> Vars;
   public:
 
     ExprCleanser(Expr *E) {
@@ -1729,6 +1725,13 @@ namespace {
           return { true, P };
         }
 
+        bool walkToDeclPre(Decl *D) override {
+          if (auto VD = dyn_cast<VarDecl>(D))
+            TS->Vars.push_back(VD);
+
+          return true;
+        }
+
         // Don't walk into statements.  This handles the BraceStmt in
         // non-single-expr closures, so we don't walk into their body.
         std::pair<bool, Stmt *> walkToStmtPre(Stmt *S) override {
@@ -1757,6 +1760,13 @@ namespace {
         if (P->hasType() && P->getType()->hasTypeVariable())
           P->setType(Type());
       }
+
+      for (auto VD : Vars) {
+        if (VD->hasType() && VD->getType()->hasTypeVariable()) {
+          VD->setType(Type());
+          VD->setInterfaceType(Type());
+        }
+      }
     }
   };
 } // end anonymous namespace
@@ -1770,6 +1780,11 @@ bool TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
                                       ConstraintSystem *baseCS) {
   PrettyStackTraceExpr stackTrace(Context, "type-checking", expr);
 
+  // First, pre-check the expression, validating any types that occur in the
+  // expression and folding sequence expressions.
+  if (preCheckExpression(expr, dc))
+    return true;
+
   // Construct a constraint system from this expression.
   ConstraintSystemOptions csOptions = ConstraintSystemFlags::AllowFixes;
   if (options.contains(TypeCheckExprFlags::PreferForceUnwrapToOptional))
@@ -1839,8 +1854,10 @@ bool TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
       return true;
   }
 
-  if (options.contains(TypeCheckExprFlags::SkipApplyingSolution))
+  if (options.contains(TypeCheckExprFlags::SkipApplyingSolution)) {
+    cleanup.disable();
     return false;
+  }
 
   // Apply the solution to the expression.
   bool isDiscarded = options.contains(TypeCheckExprFlags::IsDiscarded);

lib/Sema/TypeChecker.cpp

@@ -844,6 +844,9 @@ static Optional<Type> getTypeOfCompletionContextExpr(
                         CompletionTypeCheckKind kind,
                         Expr *&parsedExpr,
                         ConcreteDeclRef &referencedDecl) {
+  if (TC.preCheckExpression(parsedExpr, DC))
+    return None;
+
   switch (kind) {
   case CompletionTypeCheckKind::Normal:
     // Handle below.

lib/Sema/TypeChecker.h

@@ -1489,9 +1489,16 @@ class TypeChecker final : public LazyResolver {
                                      SubstitutionList SelfInterfaceSubs,
                                      SubstitutionList SelfContextSubs);
 
+  /// Pre-check the expression, validating any types that occur in the
+  /// expression and folding sequence expressions.
+  bool preCheckExpression(Expr *&expr, DeclContext *dc);
+
   /// Sets up and solves the constraint system \p cs to type check the given
   /// expression.
   ///
+  /// The expression should have already been pre-checked with
+  /// preCheckExpression().
+  ///
   /// \returns true if an error occurred, false otherwise.
   ///
   /// \see typeCheckExpression

test/IDE/complete_crashes.swift

@@ -1,6 +1,6 @@
 // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=BAD_MEMBERS_1 | %FileCheck %s -check-prefix=BAD_MEMBERS_1
 // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=BAD_MEMBERS_2 | %FileCheck %s -check-prefix=BAD_MEMBERS_2
-// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=CLOSURE_CALLED_IN_PLACE_1 | %FileCheck %s -check-prefix=WITH_GLOBAL
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=CLOSURE_CALLED_IN_PLACE_1 | %FileCheck %s -check-prefix=WITH_GLOBAL_INT
 // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RDAR_28991372 | %FileCheck %s -check-prefix=RDAR_28991372
 
 class BadMembers1 {
@@ -37,17 +37,23 @@ func badMembers2(_ a: BadMembers2) {
 
 func globalFunc() {}
 
+func globalFuncInt() -> Int { return 0 }
+
 // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LET_COMPUTED | %FileCheck %s -check-prefix=WITH_GLOBAL
 class C {
   let x : Int { #^LET_COMPUTED^# }
 }
 
 // WITH_GLOBAL: Begin completions
-// WITH_GLOBAL-DAG: Decl[FreeFunction]/CurrModule:      globalFunc()[#Void#]{{; name=.+$}}
+// WITH_GLOBAL-DAG: Decl[FreeFunction]/CurrModule: globalFunc()[#Void#]; name=globalFunc()
 // WITH_GLOBAL: End completions
 
 ({ x in 2+x })(#^CLOSURE_CALLED_IN_PLACE_1^#
 
+// WITH_GLOBAL_INT: Begin completions
+// WITH_GLOBAL_INT-DAG: Decl[FreeFunction]/CurrModule/TypeRelation[Identical]: globalFuncInt()[#Int#]; name=globalFuncInt()
+// WITH_GLOBAL_INT: End completions
+
 // rdar://19634354
 // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RDAR_19634354
 while true {

test/Interpreter/lazy_properties.swift

@@ -0,0 +1,72 @@
+// RUN: %target-run-simple-swift
+// REQUIRES: executable_test
+
+import StdlibUnittest
+
+var LazyPropertyTestSuite = TestSuite("LazyProperty")
+
+var lazyPropertyInitialized = 0
+var lazyPropertyInitialized2 = 0
+var lazyPropertyInitialized3 = 0
+
+func lazyInitFunction() -> Int {
+  lazyPropertyInitialized += 1
+  return 0
+}
+
+class LazyPropertyClass {
+  var id : Int
+  lazy var lazyProperty = lazyInitFunction()
+
+  lazy var lazyProperty2: Int = {
+    lazyPropertyInitialized2 += 1
+    return 0
+  }()
+
+  lazy var lazyProperty3: Int! = {
+    lazyPropertyInitialized3 += 1
+    return 0
+  }()
+
+  init(_ ident : Int) {
+    id = ident
+  }
+}
+
+LazyPropertyTestSuite.test("Basic") {
+  var a = LazyPropertyClass(1)
+
+  expectEqual(0, lazyPropertyInitialized)
+  _ = a.lazyProperty
+  expectEqual(1, lazyPropertyInitialized)
+  _ = a.lazyProperty
+
+  a.lazyProperty = 42   // nothing interesting happens
+
+  expectEqual(0, lazyPropertyInitialized2)
+  _ = a.lazyProperty2
+  expectEqual(1, lazyPropertyInitialized2)
+
+  a = LazyPropertyClass(2)
+
+  a = LazyPropertyClass(3)
+  a.lazyProperty = 42
+  expectEqual(1, lazyPropertyInitialized)
+
+  expectEqual(0, lazyPropertyInitialized3)
+  expectEqual(0, a.lazyProperty3)
+  expectEqual(1, lazyPropertyInitialized3)
+
+  a.lazyProperty3 = nil
+  expectEqual(nil, a.lazyProperty3)
+  expectEqual(1, lazyPropertyInitialized3)
+}
+
+// Swift 3 had a bogus 'property resetting' behavior,
+// but we don't allow that anymore.
+
+LazyPropertyTestSuite.test("Reset") {
+
+}
+
+runAllTests()

test/Interpreter/properties.swift

@@ -173,63 +173,6 @@ func test() {
 }
 test()
 
-func lazyInitFunction() -> Int {
-  print("lazy property initialized")
-  return 0
-}
-
-
-class LazyPropertyClass {
-  var id : Int
-  lazy var lazyProperty = lazyInitFunction()
-
-  lazy var lazyProperty2 : Int = {
-    print("other lazy property initialized")
-    return 0
-  }()
-
-
-  init(_ ident : Int) {
-    id = ident
-    print("LazyPropertyClass.init #\(id)")
-  }
-
-  deinit {
-    print("LazyPropertyClass.deinit #\(id)")
-  }
-  
-
-}
-
-
-func testLazyProperties() {
-  print("testLazyPropertiesStart") // CHECK: testLazyPropertiesStart
-  if true {
-    var a = LazyPropertyClass(1)      // CHECK-NEXT: LazyPropertyClass.init #1
-    _ = a.lazyProperty                // CHECK-NEXT: lazy property initialized
-    _ = a.lazyProperty    // executed only once, lazy init not called again.
-    a.lazyProperty = 42   // nothing interesting happens
-    _ = a.lazyProperty2               // CHECK-NEXT: other lazy property initialized
-
-    // CHECK-NEXT: LazyPropertyClass.init #2
-    // CHECK-NEXT: LazyPropertyClass.deinit #1
-    a = LazyPropertyClass(2)
-
-    a = LazyPropertyClass(3)
-    a.lazyProperty = 42   // Store don't trigger lazy init.
-
-    // CHECK-NEXT: LazyPropertyClass.init  #3
-    // CHECK-NEXT: LazyPropertyClass.deinit #2
-    // CHECK-NEXT: LazyPropertyClass.deinit #3
-  }
-  print("testLazyPropertiesDone")    // CHECK: testLazyPropertiesDone
-}
-
-
-
-testLazyProperties()
-
-
 
 /// rdar://16805609 - <rdar://problem/16805609> Providing a 'didSet' in a generic override doesn't work
 class rdar16805609Base<T> {

test/SILGen/decls.swift

@@ -167,22 +167,3 @@ struct StructWithStaticVar {
   init() {
   }
 }
-
-// <rdar://problem/17405715> lazy property crashes silgen of implicit memberwise initializer
-// CHECK-LABEL: // decls.StructWithLazyField.init
-// CHECK-NEXT: sil hidden @_T05decls19StructWithLazyFieldVACSiSg4once_tcfC : $@convention(method) (Optional<Int>, @thin StructWithLazyField.Type) -> @owned StructWithLazyField {
-struct StructWithLazyField {
-  lazy var once : Int = 42
-  let someProp = "Some value"
-}
-
-// <rdar://problem/21057425> Crash while compiling attached test-app.
-// CHECK-LABEL: // decls.test21057425
-func test21057425() {
-  var x = 0, y: Int = 0
-}
-
-func useImplicitDecls() {
-  _ = StructWithLazyField(once: 55)
-}
-

test/SILGen/lazy_properties.swift

@@ -0,0 +1,24 @@
+// RUN: %target-swift-frontend -Xllvm -sil-full-demangle -parse-as-library -emit-silgen -primary-file %s | %FileCheck %s
+
+// <rdar://problem/17405715> lazy property crashes silgen of implicit memberwise initializer
+// CHECK-LABEL: // lazy_properties.StructWithLazyField.init
+// CHECK-NEXT: sil hidden @_T015lazy_properties19StructWithLazyFieldVACSiSg4once_tcfC : $@convention(method) (Optional<Int>, @thin StructWithLazyField.Type) -> @owned StructWithLazyField {
+struct StructWithLazyField {
+  lazy var once : Int = 42
+  let someProp = "Some value"
+}
+
+// <rdar://problem/21057425> Crash while compiling attached test-app.
+// CHECK-LABEL: // lazy_properties.test21057425
+func test21057425() {
+  var x = 0, y: Int = 0
+}
+
+// Anonymous closure parameters in lazy initializer crash SILGen
+
+// CHECK-LABEL: sil hidden @_T015lazy_properties22HasAnonymousParametersV1xSifg : $@convention(method) (@inout HasAnonymousParameters) -> Int
+// CHECK-LABEL: sil private @_T015lazy_properties22HasAnonymousParametersV1xSifgS2icfU_ : $@convention(thin) (Int) -> Int
+
+struct HasAnonymousParameters {
+  lazy var x = { $0 }(0)
+}