Rewrite the Parameter Environments chapter
#1953
Conversation
|
I'll definitely review this tomorrow, please annoy me if I forget! |
src/param_env.md
Outdated
| // Trying to prove `T: Clone` with a `ParamEnv` of `[T: Sized]` will | ||
| // fail as there is nothing in the environment telling the trait solver | ||
| // that `T` implements `Clone`. |
There was a problem hiding this comment.
I feel like this should clarified. If we can't find anything in the ParamEnv there may still be other candidates like user-written impls. Might sound a bit nit-picky but I think mentioning this could fortify the reader's understanding. Of course, there's no such thing as impl<T> Clone for T { … } but we might want mention that somehow.
There was a problem hiding this comment.
no no this is perfect thank you for pointing this out. I remember vaguely thinking of this when writing but it must have slipped my mind thanks <3 I definitely dont want to give the impression that the only way of proving things is via the bounds in ParamEnv
src/param_env.md
Outdated
| ## What is it | ||
|
|
||
| [pe]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.ParamEnv.html | ||
| The type system relies on information in the environment in order for it to function correctly. This information is stored in the [`ParamEnv`][pe] type and it is important to use the correct `ParamEnv` when interacting with the type system. |
There was a problem hiding this comment.
Should we be more clear abt what an "environment" means here before diving into the details?
There was a problem hiding this comment.
Perhaps though I don't actually know how to do that. I'm kind of using environment here as a handwavey vibes based thing because I can't put into words what I mean concretely 😅
src/param_env.md
Outdated
| The type system relies on information in the environment in order for it to function correctly. This information is stored in the [`ParamEnv`][pe] type and it is important to use the correct `ParamEnv` when interacting with the type system. | ||
|
|
||
| For example if you have a function | ||
| The information represented by `ParamEnv` is a list of in scope where clauses, and a [`Reveal`][reveal]. A `ParamEnv` typically corresponds to a specific item's environment however it can also be created with arbitrary data that is not derived from a specific item. In most cases `ParamEnv`s are initially created via the [`param_env` query][query] which returns a `ParamEnv` derived from the provided item's where clauses. |
There was a problem hiding this comment.
Should we explain what a Reveal is too?
There was a problem hiding this comment.
I think the existing doc comments on Reveal do an adequate job of describing what the purpose of Reveal is. I added a note explicitly instructing people to click on the link if they want information.
src/param_env.md
Outdated
|
|
||
| When needing a `ParamEnv` in the compiler there generally three options for obtaining one: | ||
| - The correct env is already in scope simply use it (or pass it down the call stack to where you are) | ||
| - Call `tcx.param_env(def_id)` |
There was a problem hiding this comment.
(or param_env_reveal_all_normalized -- maybe worth mentioning and explaining why that may be desired)
Also are we forgetting ParamEnv::reveal_all()? 🤔 Useful for monomorphization usages
There was a problem hiding this comment.
Updated this section to talk about how to acquire ParamEnvs with Reveal::All and when this is desired.
src/param_env.md
Outdated
| ``` | ||
|
|
||
| [query]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ty_utils/ty/fn.param_env.html | ||
| It's very important to use the correct `ParamEnv` when interacting with the type system as otherwise it can lead to ICEs or things compiling when they shouldn't (or vice versa). See [#82159](https://github.com/rust-lang/rust/pull/82159) and [#82067](https://github.com/rust-lang/rust/pull/82067) as examples of PRs that changed rustc to use the correct param env to avoid ICE. |
There was a problem hiding this comment.
It might be useful pointing out what is the correct ParamEnv here. I understand it might be very difficult to come up with clear guidance, if so, ignore this comment :3
There was a problem hiding this comment.
We do this to some degree in the 3rd subchapter which is hopefully enough. It is pretty difficult to give general guidance here yeah 😅 hopefully what we have now is adequate
src/param_env.md
Outdated
| `[T: Sized, T: Copy, T: Clone, T: Trait, T: SuperTrait, T: SuperSuperTrait]` | ||
| This allows us to prove `T: Clone` and `T: SuperSuperTrait` when type checking `bar`. | ||
|
|
||
| The `Clone` trait has a `Sized` supertrait however we do not end up with two `T: Sized` bounds in the env (one for the supertrait and one for the implicitly added `T: Sized` bound). This is because the elaboration process (implemented via [`util::elaborate`][elaborate]) deduplicates the where clauses to avoid this. |
There was a problem hiding this comment.
The links can be very helpful 👍
|
big thanks to all of you for the reviews it was invaluable <3 |
4 participants
The current section is confusing even to some t-types members, and also generally I have spoken to a large amount of people who did not understand how ParamEnv worked at all even after reading the current section on it.
Solution: rewrite it xd
Would quite like to find someone who currently doesnt understand
ParamEnvand get them to read through this with me so I can figure out if it is actually more helpful and what problems crop up in trying to understand this ✨Also change the location of the chapter to be before most of the chapters on analysis as an understanding of what a
ParamEnvis, is pretty useful in understanding trait solving and whatnotcc @compiler-errors