Open aturon opened 8 years ago
Triage: no changes
See alexcrichton/futures-rs#479 for an example of something this would solve.
Triage: not aware of any change.
So here's a fun bug due to lacking validation:
a.rs
:
#![feature(fundamental)]
#[fundamental]
pub struct Foo;
b.rs
:
extern crate a;
impl Clone for a::Foo {
fn clone(&self) -> Self { a::Foo }
}
I think the rule we should enforce is that #[fundamental]
types have at least one type parameter, as only types can be local (e.g. constants are only ever "builtin", parameters or projections).
I think it would make more sense to say that outside the defining crate, the coherence rules for #[fundamental]
types are identical to a tuple of its generic parameters. You can't implement Clone
for Foo
for the same reason that you can't implement Clone
for ()
.
How do people feel about just stabilizing #[fundamental]
on types as-is? I'm sure there could be a "more precise" way to exert control over the coherence rules, but honestly I've only needed to do that two or three times, and in each case #[fundamental]
was sufficient.
Even if a more precise way is found, it's not the end of the world to have #[fundamental]
as well, and it would be easy to remove with an edition change.
Has anyone actually encountered a situation where more control was needed?
This needs some evaluation of what use cases will still exist after the current work on negative reasoning becomes available and stable.
It's possible there's still a use case for it after that, in which case we need to look at that more closely, and figure out if it can be supported in a reasonable way.
Or it's possible that what's left should be perma-unstable, given the coordination difficulties of doing this outside the standard library.
I have a library in https://github.com/LightningCreations/lccc (the actual implementation/host side, rather than the stdlib/target side, so I don't have privileged unstable access), xlang_abi, which contains types that are necessary for communicating between modules soundly, and includes reimplementations of some standard library types (including Box). I would like to be able to support fundamental types on, at the very least, xlang_abi::boxed::Box
(aformetioned type), as well as the Dyn* types from xlang_abi::traits (which are pointers/reference types that provide a stable abi for trait object types, mostly through emulation).
The relevant types are in https://github.com/LightningCreations/lccc/blob/main/xlang/xlang_abi/src/boxed.rs and https://github.com/LightningCreations/lccc/blob/main/xlang/xlang_abi/src/traits.rs
I have a use case for #[fundamental]
that I don't think there is much of an alternative around. I'm basically providing a wrapper type Slot<T>
. Users of the API would like to implement impl MyTrait for Slot<TheirType>
. The only way with the orphan rules is to the best of my knowledge this #[fundamental]
attribute.
FWIW, here is how miniserde
handles that: https://docs.rs/miniserde/0.1.19/miniserde/macro.make_place.html (example of an impl like the one mentioned by @mitsuhiko: https://docs.rs/miniserde/0.1.19/miniserde/de/index.html#deserializing-a-primitive)
The challenge with the approach that miniserde
does is that these types are not compatible. miniserde
gets around this by using Option<T>
as compatible public API and unsafely transmuting between &mut Option<T>
and &mut PlaceType<T>
. I'm in quite a similar situation and having one shared slot type is obviously preferrable as it could be used in public APIs consistently.
How about instead of marking the entire type as #[fundamental]
we'd only mark the specific generic parameter that allows impl
blocks in other crates?
So:
a.rs
:
#![feature(fundamental)]
pub struct Foo<#[fundamental] S, T> {
// PhantomData ceremony
}
b.rs
:
struct Bar;
// This is legal, because S is Bar and we own Bar
impl Clone for a::Foo<Bar, ()> {
fn clone(&self) -> Self {
a::Foo {
// PhantomData ceremony
}
}
}
// This is illegal, because Bar is in T. S is () and we don't own ()
impl Clone for a::Foo<(), Bar> {
fn clone(&self) -> Self {
a::Foo {
// PhantomData ceremony
}
}
}
Alternative syntax:
#[fundamental(S)]
pub struct Foo<S, T> {
// PhantomData ceremony
}
I think that the subset of #[fundamental]
-for(-generic)-types (≠ #[fundamental]
-for-traits) which could be stabilized would have to not only be explicit around not only around the generic parameters like you've suggested, @idanarye, but also around the specific traits that downstream users may implement.
Doing so would also improve the teachability of the feature as a whole: the question _what does #[fundamental]
stand for is quite complex to answer:
Stabilizing a subset of it with an explicit notation regarding both the generic parameter, and the trait involved, would thus not only improve the teachability / understandability of the feature and of its semantics, it would also improve the diagnostics when misused, as well as reduce the possitibility for footguns.
#[fundamental]
on a type, have an annotation for empty blanket impls over that typeSay, #[downstream_may_impl]
, or #[this_crate_shall_not_impl]
#[may_impl]
(or even keep the #[fundamental]
name) — that detail is left for later bikeshedding.#[bikeshed::this_crate_shall_not_impl]
impl<#[downstream] S, T> Serialize for Foo<S, T>;
this would thus not overlap with:
impl<T> Serialize for Foo<String, T> { … }
since String
can't be a #[downstream]
type, but it would overlap with
impl<S : Clone, T> Serialize for Foo<S, T> { … }
Since <S : Clone>
and <#[downstream] S>
may overlap.
Clone
be added, which it could simply not have done in the case of a #[fundamental(S)] struct Foo<S, T>
annotation (since undistinguishable from a blanket impl of Serialize
always having been present).This yields intuitive expectations w.r.t. the classic coherence rules, and is explicit over both the #[downstream]
parameter, fundamental/coherence-wise, as well as the specific traits involved.
With it, using pseudo-code, one could even finally illustrate the semantics of Box
being fundamental, for instance:
#[bikeshed::this_crate_shall_not_impl]
impl<#[downstream] T : ?Sized> * for Box<T>;
A : Allocator
parameter, since I don't even know whether Box
is fundamental over it as well or not)where we'd be able to see there is yet another magical thing with a #[fundamental]
generic type: a *
quantification over all possible traits (but for those already implemented / "shadowed"…).
This proposal would be enough to cover @mitsuhiko's and mini_serde
's use cases, for instance.
To be honest, I'd rather that impl<#[downstream] T: ?Sized>
* for
Boxbe spelt
#[downstream_may_impl] pub struct Box<#[downstream] T:
?Sized, A: Allocator>`.
On Tue, 19 Apr 2022 at 09:44, Daniel Henry-Mantilla < @.***> wrote:
I think that the subset of #[fundamental]-for(-generic)-types (≠
[fundamental]-for-traits) which could be stabilized would have to not
only be explicit around not only around the generic parameters like you've suggested, @idanarye https://github.com/idanarye, but also around the specific traits that downstream users may implement.
Doing so would also improve the teachability of the feature as a whole: the question _what does #[fundamental] stand for is quite complex to answer:
- it's either for traits, or for generic types. For a non-generic type it doesn't make sense for instance.
- In the case of generic types, it yields a universal promise, future-wise, regarding a lack of blanket impls over that generic parameter, for any trait in existence. It's not only subtle, but a huge burden maintenance-wise. For instance, breaking that promise would not be easy to spot from a library author point of view (I guess the rationale is that the stdlib has enough pairs of eyes over it so that this footgun doesn't matter).
Stabilizing a subset of it with an explicit notation regarding both the generic parameter, and the trait involved, would thus not only improve the teachability / understandability of the feature and of its semantics, it would also improve the diagnostics when misused, as well as reduce the possitibility for footguns. Instead of #[fundamental], have a #[downstream_may_impl] annotation
- Or just #[may_impl], or keep #[fundamental]. That detail is left for later bikeshedding.
[downstream_may_impl]
impl<#[downstream] S, T> Serialize for Foo<S, T>;
this would thus not overlap with:
impl
Serialize for Foo<String, T> { … } since String can't be a #[downstream] type, but it would overlap with
impl<S : Clone, T> Serialize for Foo<S, T> { … }
Since
and <#[downstream] S> may overlap.This yields intuitive expectations w.r.t. the classic coherence rules, and is explicit over both the #[downstream] parameter, fundamental/coherence-wise, as well as the specific traits involved.
With it, using pseudo-code, one coukd even finally illustrate the semantics of Box being fundamental, for instance:
[downstream_may_impl]
impl<#[downstream] T : ?Sized> * for Box
; where we'd be able to see there is yet another magical thing with a
[fundamental] generic type: a * quantification over all possible traits.
— Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/29635#issuecomment-1102675387, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGLD27YBWXG2KCF6FSMPBLVF22ELANCNFSM4BTTUBWA . You are receiving this because you commented.Message ID: @.***>
(I'm skipping the
A : Allocator
parameter, since I don't even know whetherBox
is fundamental over it as well or not)
What do you mean "as well"? With your suggestion a type can be fundamental over different generic parameters as long as it's for different traits, but it should still not be fundamental over different generic paramters for the same trait. If Box
is fundamental for both T
and A
over all the traits, than one crate can implement a trait for it for a T
that it owns and another crate can implement a trait for it for an A
that it owns, and we'd get into the same diamond pattern the orphan rules were created to prevent.
It's up for for the individual type to decide which generic parameters to be fundamental over, and in Box
's case it's an easy choice - it's T
. HashMap
has a much harder choice - should it be fundamental over K
or over V
? (probably V
, but K
has a strong enough case here to at least deserve a discussion)
Unless... did you mean you avoided doing something like that?
#[bikeshed::this_crate_shall_not_impl]
impl<#[downstream] T : ?Sized, A: Allocator> * for Box<T, A>;
If so - why not? Even if you allow this, it'd still be up for the downstream crate to include the allocator in their own impl
s, and if they do something wrong with it (can't imagine what exactly, but let's say the will) it'd be explicit (since they'd have to explicitly add a generic parameter) and if they do something wrong rustc
can probably catch it. So why prevent this possibility?
Heh, precisely because I didn't want to think about 2-ary-fundamental generic types I skipped the alloc parameter 😅 , but @idanarye it's nonetheless an interesting question; I actually find that my syntax can help explain what possible semantics #[fundamental]
for 2-ary-fundamental generic types could be.
Given:
//! `::upstream`
pub trait Trait {}
#[bikeshed::this_crate_shall_not_impl]
impl<#[downstream] T : ?Sized, #[downstream] A: Allocator> Trait for Box<T, A>;
and:
//! `::a`
struct Local;
impl<A> ::upstream::Trait for ::upstream::Box<crate::Local, A> {}
then it would error stating that this blanket-over-the-second-param impl overlaps with the upstream's:
impl<#[downstream] T : ?Sized, #[downstream] A: Allocator> Trait for Box<T, A>;
precisely because a downstream crate could write
//! `::b`
struct Local;
impl ::upstream::Trait for ::upstream::Box<::a::Local, crate::Local> {}
That is, a #[bikeshed::this_crate_shall_not_impl]
declaration has the advantage of explicitly forbidding blanket impls overlapping with it, not only within-the-same-crate (except for the impl * for
case), but also for downstream crates (even in the impl * for
case).
@chorman0773 the impl * for
was not an actual or serious proposal, it was just pseudo-code. For that case @idanarye's syntax or the one you've suggested could work 👍 (I just wanted to chime in to suggest that other "ultra explicit and restricted fundamental" syntax to target specific traits, since it would already be quite useful, and I find it quite useful as well even just when reasoning about fundamental)
That's the thing - this declaration should be rejected by the compiler:
impl<#[downstream] T : ?Sized, #[downstream] A: Allocator> Trait for Box<T, A>;
The rule should be that only one generic parameter is allowed to be #[downstream]
for each such implementation. The creator of the fundamental type has to choose: either this:
impl<#[downstream] T : ?Sized, A: Allocator> Trait for Box<T, A>;
Or this:
impl<T : ?Sized, #[downstream] A: Allocator> Trait for Box<T, A>;
Of course, with this syntax that gives downstream permissions for implementing specific traits, one could do:
impl<#[downstream] T : ?Sized, A: Allocator> Trait1 for Box<T, A>;
impl<T : ?Sized, #[downstream] A: Allocator> Trait2 for Box<T, A>;
And then we could have:
//! `::a`
struct Local;
impl<A> ::upstream::Trait1 for ::upstream::Box<crate::Local, A> {}
//! `::b`
struct Local;
impl ::upstream::Trait2 for ::upstream::Box<::a::Local, crate::Local> {}
I think improving the current behaviour of this might be a blocker for #32838, since it's not at all obvious that we want to let people implement things for Box<T, CustomAllocator>
, but it seems like that's what the #[fundamental]
currently does.
@scottmcm Only if the entire type is made fundamental. If the fundamentalness is limited to one generic parameters, as per my suggestion, then one could implement traits for Box<T, CustomAllocator>
only in the crate that defines the T
(or the trait). So you can't just implement things for Box<i32, CustomAllocator>
because you don't own i32
's definition.
We have a use case for #[fundamental]
types in turbo, where we're building an incremental computation engine.
A Vc<T>
represents the result of a computation. In order to read the underlying value T
, you must .await
the Vc<T>
because:
.await
calls to track dependencies.You can also pass the Vc<T>
itself around to create composite computations: fn op(Vc<T>, Vc<U>) -> Vc<V>
.
We want to allow users to implement inherent methods on Vc<T>
provided T
is local. From my understanding of the reference, this should already be possible, but it is not implemented.
Would it make sense to extend fundamental types to cover inherent impls as well?
See also this Zulip thread.
There's no disambiguation mechanism for conflicting things on inherent impls, so that would mean it's a breaking change to add anything, which seems too strong.
I thing the expectation will remain that such methods be added via extension traits, not inherent methods, even on fundamental
types.
Isn't it already a breaking change to add methods?
Consider this:
mod foo {
pub struct Foo;
}
mod bar {
pub trait Bar {
fn bar(&self);
}
impl Bar for super::Foo {
fn bar(&self) {
println!("Bar.bar");
}
}
}
use foo::Foo;
use bar::Bar;
fn main() {
Foo.bar();
}
If we add this to mod foo
:
impl Foo {
pub fn bar(&self) {
println!("Inherent bar");
}
}
It'll change the behavior of Foo.bar()
.
In the "Library Evolutions" RFC, this was noted as being an acceptable breaking change for the rust standard library. The Rust Semver RFC I believe also calls it acceptable in a minor release.
On Thu, Jan 26, 2023 at 14:47 Idan Arye @.***> wrote:
Isn't it already a breaking change to add methods?
Consider this:
mod foo { pub struct Foo;} mod bar { pub trait Bar { fn bar(&self); }
impl Bar for super::Foo { fn bar(&self) { println!("Bar.bar"); } }}
use foo::Foo;use bar::Bar; fn main() { Foo.bar();}
If we add this to mod foo:
impl Foo { pub fn bar(&self) { println!("Inherent bar"); }}
It'll change the behavior of Foo.bar().
— Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/29635#issuecomment-1405537306, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGLD27YXQWMFYEFGIHKZPTWULIDVANCNFSM4BTTUBWA . You are receiving this because you were mentioned.Message ID: @.***>
Sop wouldn't doing the same for #[fundamental]
and inherent methods also be acceptable?
@idanarye No. It's allowed for traits because you have to option of using UFCS to write it in a way that can't conflict no matter what other methods people add to other traits.
@scottmcm wrote:
There's no disambiguation mechanism for conflicting things on inherent impls, so that would mean it's a breaking change to add anything, which seems too strong.
This is an acceptable tradeoff for us. We won't have inherent impl<T> Vc<T>
in the crate where Vc
is defined. All inherent impls will occur in user crates, for some known U
type (impl Vc<U>
).
This feature flag, part of RFC 1023, is not intended to be stabilized as-is. But this issue tracks discussion about whether some external feature it needed. Perhaps there is a cleaner way to address the balance between negative reasoning and API evolution. See the RFC for details.