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

Cache expression types in failure diagnosis. #7481

Merged
merged 1 commit into from
Feb 15, 2017
Merged

Cache expression types in failure diagnosis. #7481

merged 1 commit into from
Feb 15, 2017

Conversation

rudkx
Copy link
Contributor

@rudkx rudkx commented Feb 15, 2017

Cache expression types after type checking smaller portions of an expression. The types need to be in the constraint system's type map when we call into other code that attempts to read the types out from the map.

@rudkx
Copy link
Contributor Author

rudkx commented Feb 15, 2017

@swift-ci Please smoke test

@rudkx
Copy link
Contributor Author

rudkx commented Feb 15, 2017

@swift-ci Please test
@swift-ci Please smoke test

@rudkx
Copy link
Contributor Author

rudkx commented Feb 15, 2017

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 3528ba7044e5c566959dba801beb36e21a19d9a5
Test requested by - @rudkx

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 3528ba7044e5c566959dba801beb36e21a19d9a5
Test requested by - @rudkx

@rudkx
Copy link
Contributor Author

rudkx commented Feb 15, 2017

@swift-ci Please smoke test

After we call into typeCheckExpression() we need to cache the
resulting types in the constraint system type map because we later
call into code that reads the types out of the type map.

Fixes rdar://problem/30376186 as well as a couple crashers.
@slavapestov
Copy link
Contributor

Would it be better if type checking a subexpression returned a new map that we can merge in instead of writing back to the expr and reading out?

@rudkx
Copy link
Contributor Author

rudkx commented Feb 15, 2017

It's crossed my mind. At the very least the places we're doing this should be refactored into common code under ConstraintSystem, and when I get more time to do more extensive refactoring I'll be doing that.

@rudkx
Copy link
Contributor Author

rudkx commented Feb 15, 2017

@swift-ci Please smoke test

@rudkx rudkx changed the title WIP: Cache expression types in failure diagnosis. Cache expression types in failure diagnosis. Feb 15, 2017
@rudkx
Copy link
Contributor Author

rudkx commented Feb 15, 2017

@swift-ci Please smoke test and merge

@slavapestov
Copy link
Contributor

Looks good -- looking forward to more crashers being fixed in this area :)

It would be nice to have a test case that's not a compiler crasher, but if it's tricky to derive one don't bother.

@rudkx
Copy link
Contributor Author

rudkx commented Feb 15, 2017

@swift-ci Please smoke test

@rudkx rudkx merged commit 713e5a2 into swiftlang:master Feb 15, 2017
@rudkx rudkx deleted the fix-rdar30376186 branch February 15, 2017 05:31
@jtbandes
Copy link
Contributor

👏

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.

4 participants