Skip to content

[NCGenerics] fix type lowering #70452

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 2 commits into from
Dec 14, 2023
Merged

[NCGenerics] fix type lowering #70452

merged 2 commits into from
Dec 14, 2023

Conversation

kavon
Copy link
Member

@kavon kavon commented Dec 13, 2023

TypeDecl::canBeNoncopyable is never going to be fully accurate. In this instance, we were incorrectly treating some types like S<T: ~Copyable>
as noncopyable, and thus non-trivial, despite them in some cases being
Copyable.

@kavon
Copy link
Member Author

kavon commented Dec 13, 2023

@swift-ci please smoke test

@kavon kavon enabled auto-merge December 13, 2023 22:24
@slavapestov
Copy link
Contributor

TypeDecl::canBeNoncopyable is never going to be fully accurate

Maybe it should be removed to avoid this kind of confusion? Also how is D->canBeNoncopyable() different than D->getDeclaredInterfaceType()->canBeNoncopyable()?

@kavon
Copy link
Member Author

kavon commented Dec 13, 2023

Maybe it should be removed to avoid this kind of confusion?

That's my plan in another PR. Asking the decl is a remnant of the old non-generic world of noncopyables and uses of it need to only be for that legacy code specifically guarded by it.

Also how is D->canBeNoncopyable() different than D->getDeclaredInterfaceType()->canBeNoncopyable()?

There's no TypeBase::canBeNoncopyable because a Type can actually give you an accurate answer, so I called it TypeBase::isNoncopyable.

@kavon kavon requested a review from tshortli as a code owner December 14, 2023 03:46
@kavon
Copy link
Member Author

kavon commented Dec 14, 2023

@swift-ci please smoke test

TypeDecl::canBeNoncopyable is never going to be fully accurate. In this
instance, we were incorrectly treating some types like `S<T: ~Copyable>`
 as noncopyable, and thus non-trivial, despite them in some cases being
 Copyable.
@kavon
Copy link
Member Author

kavon commented Dec 14, 2023

@swift-ci please smoke test

@kavon kavon merged commit 64226fb into swiftlang:main Dec 14, 2023
@kavon kavon deleted the fix-typelowering branch December 14, 2023 19:41
@@ -0,0 +1,51 @@
// RUN: %target-swift-frontend -emit-silgen -enable-experimental-feature NoncopyableGenerics -disable-availability-checking -module-name main %s | %FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

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

Because it uses an experimental feature, the test needs a REQUIRES: asserts or it will fail in non-assert builds.

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