-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Struct and enum declarations no longer go through finalizeDecl() #26159
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
Struct and enum declarations no longer go through finalizeDecl() #26159
Conversation
@swift-ci Please test |
@swift-ci Please test source compatibility |
Build failed |
Build failed |
8f0d39f
to
097df9a
Compare
@swift-ci Please test |
@swift-ci Please test source compatibility |
Build failed |
Build failed |
@swift-ci Please test compiler performance |
(Both source compat failures are UPASS because an unrelated problem was fixed on master) |
097df9a
to
e965c00
Compare
@swift-ci Please smoke test |
Instead of adding a resolveDeclSignature() call here, I'm going to live dangerously and try to only get the enum element type in the case where SIL type lowering has already computed it, that is, if the enum is not indirect. Soon this will become moot anyway because getInterfaceType() will be a request.
Since getStoredProperties() is a request that lowers lazy properties and property wrappers to their underlying storage, and SIL can validate stored property and enum element types, there's no longer any need for Sema to explicitly finalize members of structs and enums. SILGen can trigger any necessary type checkin work just by lowering a struct or enum type. Now the only remaining reason we need finalizeDecl() is adding implicit methods to classes, and synthesizing accessors for storage in classes and protocols.
Now that SILGen and IRGen can trigger type checking work, we have to run the full pipeline to get a complete picture. Thankfully everything still passes!
https://bugs.swift.org/browse/SR-9583 / rdar://problem/47097468
e965c00
to
84b0267
Compare
@swift-ci Please smoke test |
@@ -237,7 +237,7 @@ namespace { | |||
llvm_unreachable("shouldn't get an inout type here"); | |||
} | |||
RetTy visitErrorType(CanErrorType type) { | |||
llvm_unreachable("shouldn't get an error type here"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are error types wandering into SIL type lowering?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type checking is now sufficiently lazy that if a struct has a field with an invalid type, and you don't directly reference the invalid field from source, SILGen might end up lazily validating the stored properties of the struct anyway, if you nevertheless emit an expression involving a value of the struct's type.
This is fantastic! |
Do we get detection of recursive struct/enum definitions for free now from type layout? |
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.
Builds on #26119. Now that getStoredProperties() is a request, Sema no longer has to finalize struct and enum declarations.