Skip to content
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

Lazier type checking of lazy vars #9916

Merged

Conversation

slavapestov
Copy link
Member

@slavapestov slavapestov commented May 24, 2017

Fixes https://bugs.swift.org/browse/SR-2616.

rdar://problem/28313602
rdar://problem/21669808

This introduces a new TypeCheckExprFlags::SkipApplyingSolution flag,
and a ExprTypeCheckListener::foundSolution() that is invoked before
the solution is applied.

While the existing getTypeOfExpressionWithoutApplying() method is
similar, it's tailored for diagnostics and has some hacks in it.
It should be refactored to use the new mechanism I added.
@slavapestov
Copy link
Member Author

@swift-ci Please smoke test

Record the initializer type as soon as we have a solution, before
it is applied, and get the type from the constriant system instead
of from the final type checked expression.

Note that the coerceToMaterializable() was unnecessary, since we
always coerce the value to an rvalue type with coerceToType().

Eventually coerceToMaterializable() should go away.

This is mostly NFC, except using the result of simplifyType() rather
than the type of the final expression changes some diagnostics where it
appears we were previously losing sugar.

Also this accidentally fixes a crasher. Unfortunately the underlying
issue is still there (applying a solution has bugs with opened
existentials "leaking" out) -- this merely masks the problem by
getting the initializer type directly from the constriant system.
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>.
@slavapestov slavapestov force-pushed the lazier-type-checking-of-lazy-vars branch from a76555b to d8234ba Compare May 25, 2017 00:22
@slavapestov
Copy link
Member Author

@swift-ci Please smoke test

Copy link
Member

@rudkx rudkx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@@ -129,15 +129,15 @@ func defaultToAny(i: Int, s: String) {

let a2: Array = [1, "a", 3.5]
// expected-error@-1{{heterogeneous collection literal could only be inferred to '[Any]'; add explicit type annotation if this is intentional}}
let _: Int = a2 // expected-error{{value of type '[Any]'}}
let _: Int = a2 // expected-error{{value of type 'Array<Any>'}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the diagnostics changes here seem like improvements, but not this and the one below. Any idea why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because we declare 'let a2: Array', so we're only inferring generic arguments here and not the whole type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants