Generate documentation for auto-trait impls #47833
Conversation
|
Some changes occurred in HTML/CSS. |
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @GuillaumeGomez (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
Fixes #17606 |
kennytm
added
A-docs
|
Thanks a lot for this PR! Considering its size, it'll take a bit of time to get reviewed so don't worry. |
|
☔ The latest upstream changes (presumably #47738) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Sorry, forgot about it and I won't have have time before next week (FOSDEM). If anyone from @rust-lang/docs team could take a look as well, it'd be nice! |
Aaron1011
force-pushed
the
final_auto_trait
branch
from
4353edb
to
0ad7ff4
Compare
(I'm starting to read through this, but i might not get all the way through in one sitting |
|
@QuietMisdreavus: Oops, looks like I didn't fix the merge conflict properly. |
|
@QuietMisdreavus looks like the author addressed that error? |
|
We're still reviewing. It's taking some time considering the amount of code. |
src/librustc/infer/mod.rs
Outdated
| RefMut::map( | ||
| self.region_constraints.borrow_mut(), | ||
| |c| c.as_mut().expect("region constraints already solved")) | ||
| } | ||
|
|
||
| /// Clears the selection, evaluation, and projection cachesThis is useful when | ||
| /// repeatedly attemping to select an Obligation while chagning only |
src/librustc/traits/select.rs
Outdated
| intercrate: None, | ||
| inferred_obligations: SnapshotVec::new(), | ||
| intercrate_ambiguity_causes: None, | ||
| allow_negative_impls |
There was a problem hiding this comment.
Please add a comma at the end of this line.
src/librustdoc/clean/auto_trait.rs
Outdated
|
|
||
| // Due to the way projections are handled by SelectionContext, we need to run | ||
| // evaluate_predicates twice: once on the original param env, and once on the result of | ||
| // the first evaluate_predicates call |
There was a problem hiding this comment.
Since it's a sentence, please add a dot at the end.
src/librustdoc/clean/auto_trait.rs
Outdated
| // We fix the first assumption by manually clearing out all of the InferCtxt's caches | ||
| // in between calls to SelectionContext.select. This allows us to keep all of the | ||
| // intermediate types we create bound to the 'tcx lifetime, rather than needing to lift | ||
| // them between calls |
src/librustdoc/clean/auto_trait.rs
Outdated
| infcx.freshen(p) | ||
| } | ||
|
|
||
| fn evaluate_nested_obligations< |
There was a problem hiding this comment.
This function declaration is quite difficult to read. Could you put generics on the same line (if it fits) please?
src/librustdoc/clean/auto_trait.rs
Outdated
| } | ||
| } | ||
|
|
||
| // This is very simiiar to handle_lifetimes. Instead of matching ty::Region's to ty::Region's, |
There was a problem hiding this comment.
Btw:
match ty::Region's to ty::Region's
Is this repetition wanted or not?
src/librustdoc/clean/auto_trait.rs
Outdated
| // with determining if a given set up constraints/predicates *are* met, given some | ||
| // starting conditions (e.g. user-provided code). For this reason, it's easier | ||
| // to perform the calculations we need on our own, rather than trying to make | ||
| // existing inference/solver code do what we want |
| } | ||
|
|
||
| // This method calculates two things: Lifetime constraints of the form 'a: 'b, | ||
| // and region constraints of the form ReVar: 'a |
There was a problem hiding this comment.
To me, writing either of the form ReVar: 'a. or of the form ReVar: 'a .makes it look as though the . is part of the previous expression, instead of being a normal punctuation mark.
There was a problem hiding this comment.
Good point. Just ignore this comment then.
src/librustdoc/clean/auto_trait.rs
Outdated
| // all intermediate RegionVids. At the end, all constraints should | ||
| // be between Regions (aka region variables). This gives us the information | ||
| // we need to create the Generics | ||
| // |
There was a problem hiding this comment.
Why this empty comment line?
src/librustdoc/clean/auto_trait.rs
Outdated
| // Our goal is to 'flatten' the list of constraints by eliminating | ||
| // all intermediate RegionVids. At the end, all constraints should | ||
| // be between Regions (aka region variables). This gives us the information | ||
| // we need to create the Generics |
|
|
||
| for smaller in deps.smaller.iter() { | ||
| for larger in deps.larger.iter() { | ||
| match (smaller, larger) { |
There was a problem hiding this comment.
I have the feeling that this whole match could be greatly simplified. The only "unique" case is for RegionTarget::Region, otherwise they all perform the same operation. Could this code be rewritten (by writing a function which takes one argument and you apply the operation on it)? The match will then become something like this:
match (smaller, larger) {
(&RegionTarget::Region(r1), &RegionTarget::Region(r2)) => {
// the same as now
}
_ => the_function_call(),
}There was a problem hiding this comment.
I don't think this would remove much duplication - the behavior is different if the smaller region a RegionTarget::Region vs a RegionTarget::RegionVid
src/librustdoc/clean/auto_trait.rs
Outdated
| ty_to_fn: FxHashMap<Type, (Option<PolyTrait>, Option<Type>)>, | ||
| lifetime_to_bounds: FxHashMap<Lifetime, FxHashSet<Lifetime>>, | ||
| ) -> Vec<WherePredicate> { | ||
| let final_predicates = ty_to_bounds |
There was a problem hiding this comment.
This let binding is useless, just return the operations you're performing on ty_to_bounds
src/librustdoc/clean/auto_trait.rs
Outdated
| } | ||
|
|
||
| // This is an ugly hack, but it's the simplest way to handle synthetic impls without greatly | ||
| // refactorying either librustdoc or librustc. In particular, allowing new DefIds to be |
src/librustdoc/clean/inline.rs
Outdated
| @@ -120,11 +120,17 @@ pub fn load_attrs(cx: &DocContext, did: DefId) -> clean::Attributes { | |||
| cx.tcx.get_attrs(did).clean(cx) | |||
| } | |||
|
|
|||
|
|
|||
src/librustdoc/clean/inline.rs
Outdated
| @@ -249,6 +267,7 @@ pub fn build_impls(cx: &DocContext, did: DefId) -> Vec<clean::Item> { | |||
|
|
|||
| cx.populated_all_crate_impls.set(true); | |||
|
|
|||
|
|
|||
src/librustdoc/clean/inline.rs
Outdated
| items: trait_items, | ||
| polarity: Some(polarity.clean(cx)), | ||
| synthetic: false |
There was a problem hiding this comment.
Please add a comma at the end of this line.
|
@GuillaumeGomez: I messed up the submodule ref when I rebased. It should be fixed now. |
|
Ok, let's give it another try and see. @bors: r+ |
|
📌 Commit 8788179 has been approved by |
bors
added
S-waiting-on-bors
|
|
This is bizarre: From the From the Apparently, it actually is the platform that causes the issue. I'll try to compile a 32-bit build of Rust locally, and see if I can reproduce it. EDIT: Link to the build in question: https://travis-ci.org/rust-lang/rust/builds/343477652?utm_source=github_status&utm_medium=notification |
|
It turns out that my code is creating the final I believe the simplest solution is to sort the final |
This removes the implicit dependency on the iteration order of FxHashMap
|
@GuillaumeGomez: The issue should be resolved now - the failing tests now pass for me on both i686 and x86_64. |
|
Ah perfect! I was about to suggest to have a @bors: r+ |
|
📌 Commit 44d07df has been approved by |
…aumeGomez Generate documentation for auto-trait impls A new section is added to both both struct and trait doc pages. On struct/enum pages, a new 'Auto Trait Implementations' section displays any synthetic implementations for auto traits. Currently, this is only done for Send and Sync.  On trait pages, a new 'Auto Implementors' section displays all types which automatically implement the trait. Effectively, this is a list of all public types in the standard library.  Synthesized impls for a particular auto trait ('synthetic impls') take generic bounds into account. For example, a type ```rust struct Foo<T>(T) ``` will have 'impl<T> Send for Foo<T> where T: Send' generated for it. Manual implementations of auto traits are also taken into account. If we have the following types: ```rust struct Foo<T>(T) struct Wrapper<T>(Foo<T>) unsafe impl<T> Send for Wrapper<T>' // pretend that Wrapper<T> makes this sound somehow ``` Then Wrapper will have the following impl generated: ```rust impl<T> Send for Wrapper<T> ``` reflecting the fact that 'T: Send' need not hold for 'Wrapper<T>: Send' to hold Lifetimes, HRTBS, and projections (e.g. '<T as Iterator>::Item') are taken into account by synthetic impls:  However, if a type can *never* implement a particular auto trait (e.g. `struct MyStruct<T>(*const T)`), then a negative impl will be generated (in this case, `impl<T> !Send for MyStruct<T>`) All of this means that a user should be able to copy-paste a syntheticimpl into their code, without any observable changes in behavior (assuming the rest of the program remains unchanged).
Refactor auto trait handling in librustdoc to be accessible from librustc. These commits transfer some of the functionality introduced in #47833 to librustc with the intention of making the tools to work with auto traits accessible to third-party code, for example [rust-semverver](https://github.com/rust-lang-nursery/rust-semverver). Some rough edges remain, and I'm certain some of the FIXMEs introduced will need some discussion, most notably the fairly ugly overall approach to pull out the core logic into librustc, which was previously fairly tightly coupled with various bits and bobs from librustdoc. cc @Aaron1011
A new section is added to both both struct and trait doc pages.
On struct/enum pages, a new 'Auto Trait Implementations' section displays any synthetic implementations for auto traits. Currently, this is only done for Send and Sync.
On trait pages, a new 'Auto Implementors' section displays all types which automatically implement the trait. Effectively, this is a list of all public types in the standard library.
Synthesized impls for a particular auto trait ('synthetic impls') take generic bounds into account. For example, a type
will have 'impl Send for Foo where T: Send' generated for it.
Manual implementations of auto traits are also taken into account. If we have
the following types:
Then Wrapper will have the following impl generated:
reflecting the fact that 'T: Send' need not hold for 'Wrapper: Send' to hold
Lifetimes, HRTBS, and projections (e.g. '::Item') are taken into account by synthetic impls:
However, if a type can never implement a particular auto trait (e.g.
struct MyStruct<T>(*const T)), then a negative impl will be generated (in this case,impl<T> !Send for MyStruct<T>)All of this means that a user should be able to copy-paste a syntheticimpl into their code, without any observable changes in behavior (assuming the rest of the program remains unchanged).