Flip "region lattice" in RegionKind doc comment#152968
Flip "region lattice" in RegionKind doc comment#152968khyperia wants to merge 1 commit intorust-lang:mainfrom
Conversation
|
r? @jieyouxu rustbot has assigned @jieyouxu. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
@rustbot reroll |
|
r? me |
| /// function declaration. They have relationships to one another | ||
| /// determined based on the declared relationships from the | ||
| /// function. | ||
| /// Early-bound/free regions ([`RegionKind::ReEarlyParam`]) are the |
There was a problem hiding this comment.
I think this comment is out of date in that "free regions" here refers to a previously existing RegionKind::Free which got renamed to ReLateParam. This should probably say something like "Early/LateParam regions are the..."
There was a problem hiding this comment.
alternatively we did actually mean free regions in which case this includes 'static but talking explicitly about early bound regions is confusing...
There was a problem hiding this comment.
maybe this should actually be "Free regions (ReEarlyParam ReLateParam ReStatic) are the named lifetimes in scope from.." i'm not sure 😅
There was a problem hiding this comment.
"Early/LateParam regions are the named lifetimes in scope from the function declaration. They have relationships to one another and 'static based on .." is probably the most useful thing to document 🤔
There was a problem hiding this comment.
Early/LateParam regions are the named lifetimes in scope from the function declaration
late params can be anonymous, right? just gonna remove the word "named".
Also hm, ReLateParam can be constructed from HKT ReBound stuff (liberate_late_bound_regions), so they're not always "from the function declaration". Maybe flipping the sentence:
Lifetimes in scope from a function declaration are represented via ReEarlyParam/ReLateParam. They have relationships to one another and
'staticbased on the declared relationships from the function.
because all lifetimes from a function declaration are ReEarlyParam/ReLateParam, but not all ReLateParam are from a function declaration.
bbbbc92 to
8cca904
Compare
| /// | ||
| /// ## Bound Regions | ||
| /// | ||
| /// These are regions that are stored behind a binder and must be instantiated |
There was a problem hiding this comment.
I think the pre-existing comment here is wrong/wildly misleading. Early bound parameters are not ReBound. This comment implies that a ReBound can correspond to either an early or a late bound parameter but actually it just always corresponds to a lifetime from a for<'a, ...> binder
I would probably drop everything about early/late here and only talk about them in relation to the Binder type
There was a problem hiding this comment.
just pushed an edit, how's this?
8cca904 to
d0ac5b1
Compare
d0ac5b1 to
40374b4
Compare
If we assume that references take their region covariantly, and use that to define the subtyping relationship of regions,
'emptyis ⊤ (top) and'staticis ⊥ (bottom), and that'a <: 'bis defined as'a >= 'b. However, the RegionKind doc comment's "region lattice" had'staticat Top. While this is perhaps technically correct (a so-called "region lattice" is not a "type lattice"), it confused me a lot, and took me half an hour to be like "no, ok, I do understand things correctly, this region lattice is just flipped from the subtype lattice". Seems better to just have them be the same!I also took the opportunity to rewrite some parts of the comment with notes that would have helped me when starting out.
Bikesheds welcome (as long as they don't go on forever)!
For context: the comment "
'staticis Top" was written in 2013, before there was a discussion in 2014 that clarified that references take their regions covariantly, and'a <: 'bis defined as'a >= 'b.See also Zulip thread: https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/RegionKind.20rustc.20doc.20comment