Add a section on identifiers in the MIR #951
Conversation
rustbot
added
the
S-waiting-on-review
I think RalfJung and jonas-schievink would both be good reviewers. |
Yeah, I just wasn't sure if they're open to reviewing dev-guide PRs. |
|
That's weird... CI passed but there's a broken link: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/mir/struct.GlobalId.html cc @mark-i-m since you know more about linkcheck :) |
|
It might be due to caching. I wonder if we can turn that off now... |
|
we do have wg-mir-opt that you can (I think you can?) ping, but I have no problem getting pinged and potentially forwarding to someone else. |
|
@oli-obk Could you re-review this? I think it's ready! |
There was a problem hiding this comment.
As someone who's never really touched MIR, this makes things clearer to me! ❤️
I think it would be worth it to also mention how to use those identifiers (i.e. get the XData with X through Body::x), but I'm not really sure...
In the HIR part of the chapter I had only written:
These identifiers can be converted into one another through the [HIR map][map].
See the [HIR chapter][hir-map] for more detailed information.
|
cc @robinhundt |
Co-authored-by: Léo Lanteri Thauvin <leseulartichaut@gmail.com>
src/identifiers.md
Outdated
| - [`Promoted`] identifies a promoted constant within another item (related to | ||
| const evaluation). Note: it is unique only locally within the item, so it should be associated with a `DefId`; | ||
| [`GlobalId`] must be used if you need to *globally* identify the promoted. |
There was a problem hiding this comment.
@oli-obk The change you applied confuses me: the text now says
Note: it is unique only locally within the item, so it should be associated with a
DefId;
[GlobalId] must be used if you need to globally identify the promoted.
So which one is it? It says that you should associate it with a DefId, but it also says use GlobalId.
There was a problem hiding this comment.
GlobalId is the combination of DefId + Substs + Option<Promoted>. You can identify a promoted with just a DefId and Promoted, but you may not be able to compute its value without the Substs, because it's still polymorphic.
There was a problem hiding this comment.
So what should I change this sentence to?
There was a problem hiding this comment.
I guess something like must be used if you need a globally unique identifier for the monomorphic version of a promoted? Is that filled with too many weird words or do you think someone who gets to this section the first time will be able to understand it?
There was a problem hiding this comment.
Hmm, but didn't you say that you can identify a promoted with just DefId and Promoted, and you only need GlobalId in order to actually compute the value?
There was a problem hiding this comment.
My mental model was always that GlobalId identifies the monomorphic version of sth, DefId identifies a definition (potentially polymorphic) and (DefId, Promoted) identifies a promoted within a definition (also potentially polymorphic). But "identifying a monomorphic version" may not really be a thing...
There was a problem hiding this comment.
You know what, how about we just say "GlobalId gives you more information (TODO)" and come back to this later? That way we can get the rest of it merged :)
There was a problem hiding this comment.
I personally wouldn't mind, especially given that this PR itself removes a giant TODO :)
|
Thanks for your work on this @camelid :) |
r? @oli-obk
By the way, let me know if there are other people I can ping for review so I
don't assign every MIR PR to you :)
@rustbot modify labels: waiting-on-review