Skip to content

isInvalid() Is Finally Semantic #27922

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 6 commits into from
Oct 30, 2019
Merged

isInvalid() Is Finally Semantic #27922

merged 6 commits into from
Oct 30, 2019

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Oct 29, 2019

The isInvalid() bit is intended to be sync'd to the presence of an ErrorType in the interface type of a ValueDecl (for a regular Decl, it's kinda higgledy-piggledy). Historically, this had to be handled in an ad-hoc manner and callers had to make sure to set both lest they leave invalid ASTs in an inconsistent state. We even went so far as to teach the constraint solver to reset the invalid bit when it was unwinding which meant we had to track every single ValueDecl.

Instead, make this predicate ask for the interface type. The setter now handles setting the interface type as well. A lot of cleanup falls out of this one change:

  • No more !getInterfaceType() || isInvalid() dances
  • No more PossiblyInvalidDecls
  • No more setInvalid(); setInterfaceType(ErrorType::get())
  • A lot less variable 'x' is not bound by any pattern because we're actually invalidating the AST correctly!

A few places had to be patched up to break the new cycles this introduces, but serendipitously the cycle breaker in getUnopenedTypeOfReference appears to be unnecessary.

@CodaFi CodaFi requested a review from slavapestov October 29, 2019 01:34
@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 29, 2019

@swift-ci please test

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 29, 2019

@swift-ci please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - dd13746a26ef0e1efdbc5c9db0a0c77891189d26

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 29, 2019

@swift-ci please smoke test macOS platform

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 29, 2019

@swift-ci Please Test Source Compatibility Release

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 29, 2019

@swift-ci please smoke test

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 29, 2019

Love 2 have my build ICE'd

@swift-ci Please Test Source Compatibility Release

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 30, 2019

@swift-ci Please Test Source Compatibility Release

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 30, 2019

@swift-ci please test

@swiftlang swiftlang deleted a comment from swift-ci Oct 30, 2019
@swiftlang swiftlang deleted a comment from swift-ci Oct 30, 2019
@swiftlang swiftlang deleted a comment from swift-ci Oct 30, 2019
@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 30, 2019

@swift-ci please clean test macOS platform

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 30, 2019

@swift-ci Please Test Source Compatibility Release

@CodaFi
Copy link
Contributor Author

CodaFi commented Oct 30, 2019

This finally passes and naturally there's a merge conflict...

Now that setInvalid() is a semantic property, drop the explicit calls to
reset the interface type.  It is already ErrorType.
Patch up all the places that are making a syntactic judgement about the
isInvalid() bit in a ValueDecl.  They may continue to use that query,
but most guard themselves on whether the interface type has been set.
@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

⛵️

@CodaFi CodaFi merged commit 7b90628 into swiftlang:master Oct 30, 2019
@CodaFi CodaFi deleted the bitflip branch October 30, 2019 23:31
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