-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Elaborate destruct host effect clauses with structurally implied clauses #144856
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
base: master
Are you sure you want to change the base?
Conversation
r? @SparrowLii rustbot has assigned @SparrowLii. Use |
@@ -2147,9 +2147,7 @@ const fn expect_failed(msg: &str) -> ! { | |||
#[rustc_const_unstable(feature = "const_try", issue = "74935")] | |||
impl<T> const Clone for Option<T> | |||
where | |||
// FIXME(const_hack): the T: ~const Destruct should be inferred from the Self: ~const Destruct in clone_from. | |||
// See https://github.com/rust-lang/rust/issues/144207 | |||
T: ~const Clone + ~const Destruct, |
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.
I'm confused how a bound on clone_from
can mean we remove a bound from clone
. That seems backwards, since this function doesn't call clone_from
?
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.
I guess this might be okay since this bound was on impl
, not fn clone
?
cc @rust-lang/project-const-traits this behavior definitely needs to be noted somewhere, and I'm not totally sure if it makes sense to imply by default, because it may have some unnecessary semver observable implications. |
), | ||
); | ||
|
||
// `Adt: [const] Trait` implies each field also implements `[const] Trait` |
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.
If we did apply this, I'd probably want to abstract out the function that maps from a ty to its constituent types and share it between the Destruct
trait logic and here. (T,): Destruct
should probably imply T: Destruct
, for example.
And [Ty; N]: Destruct
should probably imply Ty: Destruct
(perhaps except for 0, but that's another problem with the destruct trait...).
Logic would probably need to be pulled out of https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_next_trait_solver/solve/assembly/structural_traits.rs.html#743-820
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.
Oh, absolutely I should do so and fill up missing cases for arrays and tuples. Thanks!
And it seems that I should also check for whether ManuallyDrop<T>: Destruct
bound accidentally implies the bound T: Destruct
as well 😅
I've thought more about this, and I believe there might be semver-breaking implications. For example: // crate A
pub struct Foo<T> {
inner: Bar<T>,
// ...
}
// crate B, depends on crate A
const fn foobar<T>(foo: Foo<T>, bar: Bar<T>)
where
Foo<T>: [const] Destruct,
{
// Some code relying on the implied bound: `Bar<T>: [const] Destruct`
} If crate A removes the private field And I think the inverse can also happen with the current nightly rustc. For instance: If crate A deletes private field And I think the opposite direction might be happen in current nightly rustc, like: pub struct Foo {
// All fields currently satisfy `const Destruct`
// ...
} If a private field that does not implement This suggests we might need to restrict structural Destruct inference to public fields only, to avoid semver breakage from purely private structural changes. 🤔 |
I think we'd probably want to resolve this at the most restricted scope, i.e. when comparing impl'd method bounds with trait method bounds. I'm imagining us doing some sort of special elaboration for This elaboration should also be gated behind a separate feature as it will probably have to stabilize separately to const traits |
That sounds like the right approach to address the semver issue. Thanks! I’ll give it a try.
|
Fixes #144207