Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 26 additions & 9 deletions lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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))
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions lib/Sema/TypeChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions lib/Sema/TypeChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions test/IDE/complete_crashes.swift
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
72 changes: 72 additions & 0 deletions test/Interpreter/lazy_properties.swift
Original file line number Diff line number Diff line change
@@ -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()
57 changes: 0 additions & 57 deletions test/Interpreter/properties.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
19 changes: 0 additions & 19 deletions test/SILGen/decls.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

24 changes: 24 additions & 0 deletions test/SILGen/lazy_properties.swift
Original file line number Diff line number Diff line change
@@ -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)
}