Skip to content

Pre-Request Cleanup For Implicit Constructors #27978

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

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Oct 30, 2019

Push through an easy refactoring to the way we validate and install
implicit constructors. This patch would be NFC but for a regression
test that now must diagnose. #26159 changed validation order in such
a way that the code in validation-test-macosx-x86_64/compiler_crashers_2_fixed/0124-sr5825.swift
used to be accepted. This patch once again changes validation order, so
we now reject this code, restoring the behavior seen on all prior
versions of Swift.

On its face, this test should work. In order for it to do so, witness
matching has to be smarter about the declarations it asks for their
interface type, or it will risk these circular constructions
accidentally being accepted or rejected on a whim.

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 30, 2019

@swift-ci please smoke test

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 30, 2019

@swift-ci please test source compatibility

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 31, 2019

Blargh, I moved this cycle again apparently.

@hamishknight
Copy link
Contributor

Ah, I think the if (!ctor->isInvalid()) { check is now also triggering the interface type computation since #27922.

@slavapestov
Copy link
Contributor

@hamishknight You can try just removing that check.

@CodaFi CodaFi force-pushed the on-the-construction-of-small-universes branch from b2f5d3d to e3cbb4b Compare October 31, 2019 02:42
@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 31, 2019

@swift-ci please smoke test

Push through an easy refactoring to the way we validate and install
implicit constructors.  This patch would be NFC but for a regression
test that now must diagnose.  swiftlang#26159 changed validation order in such
a way that the code in validation-test-macosx-x86_64/compiler_crashers_2_fixed/0124-sr5825.swift
used to be accepted.  This patch once again changes validation order, so
we now reject this code, restoring the behavior seen on all prior
versions of Swift.

On its face, this test should work.  In order for it to do so, witness
matching has to be smarter about the declarations it asks for their
interface type, or it will risk these circular constructions
accidentally being accepted or rejected on a whim.
@CodaFi CodaFi force-pushed the on-the-construction-of-small-universes branch from e3cbb4b to 0af3434 Compare October 31, 2019 20:46
@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 31, 2019

@swift-ci please smoke test

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 31, 2019

Neat

⛵️

@CodaFi CodaFi merged commit 2436447 into swiftlang:master Oct 31, 2019
@CodaFi CodaFi deleted the on-the-construction-of-small-universes branch October 31, 2019 22:27
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