search

rust-lang / types-team

Home of the "types team", affiliated with the compiler and lang teams.
https://rust-lang.github.io/types-team/
Apache License 2.0
94 stars 19 forks source link

Rename some `RegionKind` variants to be inline with `TyKind`/`ConstKind` #95

Closed BoxyUwU closed 10 months ago

BoxyUwU commented 1 year ago

Proposal

I would like to rename: RegionKind::EarlyBound to RegionKind::EarlyParam RegionKind::Free to RegionKind::LateParam RegionKind::LateBound to RegionKind::Bound

I find this to be far more consistent with TyKind/ConstKind. It's taken me a while for it to properly stick what all the 3 variants are on RegionKind since they're so differently named than TyKind, and also have an entire extra variant for representing lifetime parameters. I think the meaning of RegionKind::Free would have been a lot clearer with RegionKind::LateParam.

I also think it's kind of confusing that RegionKind::EarlyBound gets used for early bound generic paramaters but we do not use RegionKind::LateBound for late bound generic parameters. I think its unnecessarily confusing to draw a parallel between ReLateBound/ReEarlyBound and the concept of late/early bound generic parameters so directly when it does not actually apply. I am comfortable with drawing the parallel between EarlyParam/LateParam since it does not actually say Early/Late bound and in this case the parallel is actually present.

I would also like to avoid drawing a similarity between generic parameters and bound vars right now since we do not actually represent params with bound vars right now (and if we did we'd have to rename RegionKind::LateBound to RegionKind::Bound anyway).

Renaming EarlyBound/Free is it a bit of a wasted effort if we end up removing them in favor of placeholders and boundvars but this seems really simple to do wheras it's probably more complex to replace our param representation with bound vars as was briefly discussed in the t-types meeting on early/late bound parameters.

I do not work with regions very much since I don't touch borrowck much. I'd feel much more comfortable if someone who has spent a fair amount of time working with regions would approve this or say this is a coherent naming scheme for the regions.

Mentors or Reviewers

I can review this or implement this or mentor this. I think really anybody on t-compiler-contributors or t-types is qualified to do any of those 3 things.

Process

The main points of the Major Change Process are as follows:

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

rustbot commented 1 year ago

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/types

compiler-errors commented 1 year ago

@rustbot second

rustbot commented 1 year ago

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/types

compiler-errors commented 1 year ago

yeet

@rustbot second

compiler-errors commented 1 year ago

cc https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/naming.20bikeshedding/near/386440383

WaffleLapkin commented 10 months ago

Should this be considered accepted? the "second" comment was 2 months ago

BoxyUwU commented 10 months ago

Yeah it should lol