Skip to content

Fix exponential type checking of tuple literals. #15419

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
merged 1 commit into from
Mar 27, 2018
Merged

Fix exponential type checking of tuple literals. #15419

merged 1 commit into from
Mar 27, 2018

Conversation

rudkx
Copy link
Contributor

@rudkx rudkx commented Mar 22, 2018

This fixes two easy cases where we would go exponential in type
checking tuple literals.

Instead of generating a conversion to a single type variable (which
results in one large constraint system), we generate a conversion ot
the same type that appears in the initializer expression (which for
tuples is a tuple type, which naturally splits the constraint system).

I experimented with trying to generalize this further, but ran into
problems getting it working, so for now this will have to do.

Fixes rdar://problem/20233198.

@rudkx
Copy link
Contributor Author

rudkx commented Mar 22, 2018

@swift-ci Please smoke test

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Nice! Very easy win

@rudkx
Copy link
Contributor Author

rudkx commented Mar 26, 2018

@swift-ci Please smoke test

@rudkx
Copy link
Contributor Author

rudkx commented Mar 26, 2018

Oops, need to move the fixed perf test over as well.

This fixes two easy cases where we would go exponential in type
checking tuple literals.

Instead of generating a conversion to a single type variable (which
results in one large constraint system), we generate a conversion ot
the same type that appears in the initializer expression (which for
tuples is a tuple type, which naturally splits the constraint system).

I experimented with trying to generalize this further, but ran into
problems getting it working, so for now this will have to do.

Fixes rdar://problem/20233198.
@rudkx
Copy link
Contributor Author

rudkx commented Mar 26, 2018

@swift-ci Please smoke test

@rudkx
Copy link
Contributor Author

rudkx commented Mar 26, 2018

@swift-ci Please smoke test Linux platform

@rudkx
Copy link
Contributor Author

rudkx commented Mar 26, 2018

@swift-ci Please clean smoke test Linux platform

@rudkx
Copy link
Contributor Author

rudkx commented Mar 27, 2018

@swift-ci Please test source compatibility

1 similar comment
@rudkx
Copy link
Contributor Author

rudkx commented Mar 27, 2018

@swift-ci Please test source compatibility

@rudkx
Copy link
Contributor Author

rudkx commented Mar 27, 2018

Source compat failure is also happening without my changes.

@rudkx rudkx merged commit 0da0d10 into swiftlang:master Mar 27, 2018
@rudkx rudkx deleted the faster-typecheck-tuples branch March 27, 2018 06:13
@davezarzycki
Copy link
Contributor

Hi @rudkx – A question for you about the changes to the PatternKind::Named case in CSGen.cpp. Is it fair to say that isWeak really means "requires a wrapped type"? In the case of 'weak', that means Optional, of course. I'm asking because I have a branch with two additional ReferenceOwnership enumerations, and I'm trying to understand why 'weak' is special, but not 'unowned' or 'unmanaged'. Thanks!

@rudkx
Copy link
Contributor Author

rudkx commented Mar 27, 2018

As I understand it, weak variables must be optional because they are reference types, but do not form strong references; i.e. the object they refer to can be released as a result of all the strong references going out of scope (or being released). When that happens, they are nil'ed out.

@davezarzycki
Copy link
Contributor

I understand how weak references work. Can you just review #15540 instead? Thanks! :-)

davezarzycki added a commit to davezarzycki/swift that referenced this pull request Mar 28, 2018
davezarzycki added a commit that referenced this pull request Mar 28, 2018
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.

3 participants