Rewrite the Parameter Environments chapter#1953
Conversation
|
I'll definitely review this tomorrow, please annoy me if I forget! |
| // 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
compiler-errors
left a comment
compiler-errors
left a comment
There was a problem hiding this comment.
i was partly through leaving some reviews when fmease came thru, some of these may be not relevant anymore but im putting them up so they dont bitrot (e.g. if lcnr also reviews tomorrow while i am asleep haha)
| ## 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 😅
| 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.
compiler-errors
left a comment
compiler-errors
left a comment
There was a problem hiding this comment.
(some other random comments when i was reading from the bottom up, will review the middle parts tomorrow 😸)
|
|
||
| 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.
jieyouxu
left a comment
jieyouxu
left a comment
There was a problem hiding this comment.
As someone who is :ferrisClueless: about typeck and ParamEnv, this looks great to me and is a very clear explanation on what ParamEnv is / what it's used for. Pointing out the pitfalls is also very 😎.
| ``` | ||
|
|
||
| [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
| `[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