[rustdoc-json] Show whether ?Sized parameters are actually Sized
#143559
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,8 +37,8 @@ pub type FxHashMap<K, V> = HashMap<K, V>; // re-export for use in src/librustdoc | |
| // will instead cause conflicts. See #94591 for more. (This paragraph and the "Latest feature" line | ||
| // are deliberately not in a doc comment, because they need not be in public docs.) | ||
| // | ||
| // Latest feature: Add `allow_unsized` field to `GenericParamDefKind::Type` | ||
| pub const FORMAT_VERSION: u32 = 56; | ||
|
|
||
| /// The root of the emitted JSON blob. | ||
| /// | ||
|
|
@@ -977,6 +977,11 @@ pub enum GenericParamDefKind { | |
| /// is bound by `Trait`) is synthetic, because it was not originally in | ||
| /// the Rust source text. | ||
| is_synthetic: bool, | ||
| /// Whether this type parameter can be instantiated with an unsized type. | ||
| /// | ||
| /// This is `true` if the parameter has a `?Sized` bound without any | ||
| /// additional bounds that imply `Sized`. | ||
| allow_unsized: bool, | ||
|
Comment on lines
+980
to
+984
There was a problem hiding this comment. This flag doesn't scale to associated types. Relaxed bounds are not just permitted on type parameters but also in the so-called item bounds of associated types. How would you represent allow unsized for pub trait Trait { type Type: Bound + ?Sized; }
// ^^^^^^ effective or ineffective?
pub trait Bound/*: Sized*/ {}There was a problem hiding this comment. Side note: This flag doesn't scale to (A) Sized Hierarchy (disclaimer: the RFC is still unapproved; impl unstable+incomplete) which gets rid the imprecise notion of "unsized" or (B) to the addition of other default traits apart from There was a problem hiding this comment. This flag doesn't scale to existential impl-Trait like RPIT(IT)s. How would you represent allow unsized for pub fn make() -> Box<impl Bound + ?Sized> {
// ^^^^^^ effective or ineffective?
Box::new([]) as Box<[_]> // (1)
//Box::new([]) // (2)
}
pub trait Bound {} // (1)
impl Bound for [u8] {} // (1)
//pub trait Bound: Sized {} // (2)
//impl<const N: usize> Bound for [u8; N] {} // (2)There was a problem hiding this comment. Given these shortcomings, I would strongly advise against taking this approach / representation, even for an MVP. Moreover, I know that your goal is to make rustdoc identify arbitrary redundant bounds (e.g., the The obvious alternative approach would be to add a There was a problem hiding this comment.
I'd say that's "nice to have" at best. With our current approach for finding type-related breakage, this would likely be a nice optimization but not more than that. Happy to discuss this in more detail if useful!
In light of the above, I think this is nice to have but not required. I'm open to reworking the scheme as the language and our needs evolve, for example if the sized hierarchy proposal goes through.
I'm not sure I quite understand this proposal. In the case of In the case of Assuming so, I'm afraid I don't know how to have rustc determine whether a bound is redundant in the general case. This PR is already at the limit of my understanding of the rustc internals 😅 If you're open to driving that approach forward, I'd be thrilled to use it. But if you don't have cycles for it, I'd prefer something more limited that unblocks this work — even if we know we need to extend or tweak it in the future. Having rustdoc JSON be unstable gives us that freedom, and I'd like us to use it. There was a problem hiding this comment. Respectfully, don't overindex on the "Copy+Clone" case1 or the word "redundant". In the examples I used the word "ineffective" instead but that also doesn't quite generalize. Just pick "bikeshed_property" in your head. What's far more important is the assoc ty bounds case and the existential impl-Trait (RPIT(IT), TAIT, ATPIT) case which demonstrates quite clearly that a Boolean flag on ty param defs simply doesn't model the domain sufficiently / correctly. I'm sorry to say, but I simply won't accept that. It's a property of an individual bound, that's what I was trying to get at. Footnotes
There was a problem hiding this comment. Apologies — I think I did a poor job explaining. Let me try again. I believe that the case of "explicitly user-written bounds contain bounds already implied by any supertrait bounds" is merely an optimization and not important for cargo-semver-checks. This encompasses any Implied bound relaxations like
I agree with this. I failed to mention it last time, but I'm happy to extend the approach to cover assoc ty and existential cases. Those just weren't on my radar when I opened this PR, so thanks for bringing them up! |
||
| }, | ||
|
|
||
| /// Denotes a constant parameter. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| #![crate_name = "allow_unsized"] | ||
|
|
||
| use std::fmt::Debug; | ||
| use std::marker::PhantomData; | ||
|
|
||
| pub trait CustomSized: Sized {} | ||
| impl CustomSized for u8 {} | ||
|
There was a problem hiding this comment. (unused trait impl as far as I can tell) |
||
|
|
||
| // Generic functions | ||
| //@ is "$.index[?(@.name=='func_custom')].inner.function.generics.params[0].kind.type.allow_unsized" false | ||
| pub fn func_custom<T: ?Sized + CustomSized>() {} | ||
|
There was a problem hiding this comment. This should test that we don't remove the |
||
|
|
||
| //@ is "$.index[?(@.name=='func_custom')].inner.function.generics.params[0].kind.type.allow_unsized" false | ||
| pub fn func_custom_where_denies<T: ?Sized>() | ||
| where | ||
| T: CustomSized, | ||
| { | ||
| } | ||
|
|
||
| //@ is "$.index[?(@.name=='func_custom')].inner.function.generics.params[0].kind.type.allow_unsized" false | ||
| pub fn func_custom_where_allows<T: CustomSized>() | ||
| where | ||
| T: ?Sized, | ||
| { | ||
| } | ||
|
|
||
| //@ is "$.index[?(@.name=='func_custom')].inner.function.generics.params[0].kind.type.allow_unsized" false | ||
| pub fn func_custom_where_both<T>() | ||
| where | ||
| T: ?Sized + CustomSized, | ||
| { | ||
| } | ||
|
|
||
| //@ is "$.index[?(@.name=='func_unsized')].inner.function.generics.params[0].kind.type.allow_unsized" true | ||
| pub fn func_unsized<T: ?Sized>() {} | ||
|
|
||
| //@ is "$.index[?(@.name=='func_clone')].inner.function.generics.params[0].kind.type.allow_unsized" false | ||
| pub fn func_clone<T: ?Sized + Clone>() {} | ||
|
|
||
| //@ is "$.index[?(@.name=='func_debug')].inner.function.generics.params[0].kind.type.allow_unsized" true | ||
| pub fn func_debug<T: ?Sized + Debug>() {} | ||
|
|
||
| // Generic structs | ||
| //@ is "$.index[?(@.name=='StructCustom')].inner.struct.generics.params[0].kind.type.allow_unsized" false | ||
| pub struct StructCustom<T: ?Sized + CustomSized>(PhantomData<T>); | ||
|
|
||
| //@ is "$.index[?(@.name=='StructUnsized')].inner.struct.generics.params[0].kind.type.allow_unsized" true | ||
| pub struct StructUnsized<T: ?Sized>(PhantomData<T>); | ||
|
|
||
| //@ is "$.index[?(@.name=='StructClone')].inner.struct.generics.params[0].kind.type.allow_unsized" false | ||
| pub struct StructClone<T: ?Sized + Clone>(PhantomData<T>); | ||
|
|
||
| //@ is "$.index[?(@.name=='StructDebug')].inner.struct.generics.params[0].kind.type.allow_unsized" true | ||
| pub struct StructDebug<T: ?Sized + Debug>(PhantomData<T>); | ||
|
|
||
| // Struct with `?Sized` bound, and impl blocks that add additional bounds | ||
| //@ is "$.index[?(@.name=='Wrapper')].inner.struct.generics.params[0].kind.type.allow_unsized" true | ||
| pub struct Wrapper<T: ?Sized>(PhantomData<T>); | ||
|
|
||
| //@ is "$.index[?(@.docs=='impl custom')].inner.impl.generics.params[0].kind.type.allow_unsized" false | ||
| /// impl custom | ||
| impl<T: ?Sized + CustomSized> Wrapper<T> { | ||
| pub fn impl_custom() {} | ||
| } | ||
|
|
||
| //@ is "$.index[?(@.docs=='impl clone')].inner.impl.generics.params[0].kind.type.allow_unsized" false | ||
| /// impl clone | ||
| impl<T: ?Sized + Clone> Wrapper<T> { | ||
| pub fn impl_clone() {} | ||
| } | ||
|
|
||
| //@ is "$.index[?(@.docs=='impl debug')].inner.impl.generics.params[0].kind.type.allow_unsized" true | ||
| /// impl debug | ||
| impl<T: ?Sized + Debug> Wrapper<T> { | ||
| pub fn impl_debug() {} | ||
| } | ||
|
|
||
| impl<T: ?Sized> Wrapper<T> { | ||
| //@ is "$.index[?(@.name=='assoc_custom')].inner.function.generics.params[0].kind.type.allow_unsized" false | ||
| pub fn assoc_custom<U: ?Sized + CustomSized>(&self) {} | ||
|
|
||
| //@ is "$.index[?(@.name=='assoc_unsized')].inner.function.generics.params[0].kind.type.allow_unsized" true | ||
| pub fn assoc_unsized<U: ?Sized>(&self) {} | ||
|
|
||
| //@ is "$.index[?(@.name=='assoc_clone')].inner.function.generics.params[0].kind.type.allow_unsized" false | ||
| pub fn assoc_clone<U: ?Sized + Clone>(&self) {} | ||
|
|
||
| //@ is "$.index[?(@.name=='assoc_debug')].inner.function.generics.params[0].kind.type.allow_unsized" true | ||
| pub fn assoc_debug<U: ?Sized + Debug>(&self) {} | ||
| } | ||
|
|
||
| // Traits with generic parameters | ||
| //@ is "$.index[?(@.name=='TraitCustom')].inner.trait.generics.params[0].kind.type.allow_unsized" false | ||
| pub trait TraitCustom<T: ?Sized + CustomSized> {} | ||
|
|
||
| //@ is "$.index[?(@.name=='TraitUnsized')].inner.trait.generics.params[0].kind.type.allow_unsized" true | ||
| pub trait TraitUnsized<T: ?Sized> {} | ||
|
|
||
| //@ is "$.index[?(@.name=='TraitClone')].inner.trait.generics.params[0].kind.type.allow_unsized" false | ||
| pub trait TraitClone<T: ?Sized + Clone> {} | ||
|
|
||
| //@ is "$.index[?(@.name=='TraitDebug')].inner.trait.generics.params[0].kind.type.allow_unsized" true | ||
| pub trait TraitDebug<T: ?Sized + Debug> {} | ||
|
|
||
| pub trait TraitMethods { | ||
| //@ is "$.index[?(@.name=='method_custom')].inner.function.generics.params[0].kind.type.allow_unsized" false | ||
| fn method_custom<T: ?Sized + CustomSized>(); | ||
|
|
||
| //@ is "$.index[?(@.name=='method_unsized')].inner.function.generics.params[0].kind.type.allow_unsized" true | ||
| fn method_unsized<T: ?Sized>(); | ||
|
|
||
| //@ is "$.index[?(@.name=='method_clone')].inner.function.generics.params[0].kind.type.allow_unsized" false | ||
| fn method_clone<T: ?Sized + Clone>(); | ||
|
|
||
| //@ is "$.index[?(@.name=='method_debug')].inner.function.generics.params[0].kind.type.allow_unsized" true | ||
| fn method_debug<T: ?Sized + Debug>(); | ||
| } | ||
|
|
||
| // `where` clauses on trait functions, which only affect `T` for that function | ||
| //@ is "$.index[?(@.name=='OuterDebug')].inner.trait.generics.params[0].kind.type.allow_unsized" true | ||
| pub trait OuterDebug<T: ?Sized> { | ||
| fn foo() | ||
| where | ||
| T: Debug; | ||
| } | ||
|
|
||
| //@ is "$.index[?(@.name=='OuterClone')].inner.trait.generics.params[0].kind.type.allow_unsized" true | ||
| pub trait OuterClone<T: ?Sized> { | ||
| fn foo() | ||
| where | ||
| T: Clone; | ||
| } | ||
|
|
||
| // Synthetic generic parameters | ||
| //@ is "$.index[?(@.name=='synth_clone')].inner.function.generics.params[0].kind.type.allow_unsized" false | ||
| pub fn synth_clone(_: impl Clone + ?Sized) {} | ||
|
|
||
| //@ is "$.index[?(@.name=='synth_debug')].inner.function.generics.params[0].kind.type.allow_unsized" true | ||
| pub fn synth_debug(_: impl Debug + ?Sized) {} | ||
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.
@fmease would you mind reviewing this bit? I'm not familiar enough with how rustc/rustdoc represent generic bounds to be confident saying this is correct.