Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: rename a bunch of things #27167

Open
josharian opened this Issue Aug 23, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@josharian
Copy link
Contributor

josharian commented Aug 23, 2018

Over time, I've noticed many things in the compiler that I'd like to rename, but I haven't because of the merge pain it causes and the review annoyance. I propose to use this issue to accumulate (and bikeshed) a list of renamings that we as a community want. Then a trusted contributor can do a single, automated bulk rename all in one commit.

I think this should happen shortly after the freeze starts. That gives everyone maximum opportunity to get all their CLs submitted. (Doing it early in the cycle is the worst idea, as it potentially breaks three months of private CL work.)

I'll start off the list with:

OPROC -> OGO

I'd also like new names for all of Etop, Erv, Etype, Ecall, Efnstruct, Easgn, and Ecomplit, but don't have good suggestions.

@josharian josharian added this to the Go1.12 milestone Aug 23, 2018

@randall77

This comment has been minimized.

Copy link
Contributor

randall77 commented Aug 24, 2018

OCOM -> OCOMPLEMENT. I always parse the former as COMmunicate.
OMINUS -> ONEGATE. It's not symmetric with OPLUS anymore, but OPLUS is rare.
OIND -> ODEREF. I'd be happy with a better replacement, but OIND is weak.

There's a lot of stuff in the compiler that could be camel-cased instead of all lowercase.

I want to change the sense of kindNoPointers. That's a bit more than just a rename, so maybe that's a separate change.

@josharian

This comment has been minimized.

Copy link
Contributor Author

josharian commented Nov 6, 2018

@gopherbot

This comment has been minimized.

Copy link

gopherbot commented Nov 18, 2018

Change https://golang.org/cl/150140 mentions this issue: cmd/compile: bulk rename

@gopherbot gopherbot closed this in 0b9004f Nov 19, 2018

bradfitz pushed a commit that referenced this issue Nov 21, 2018

cmd/compile: bulk rename
This change does a bulk rename of several identifiers in the compiler.
See #27167 and https://docs.google.com/document/d/19_ExiylD9MRfeAjKIfEsMU1_RGhuxB9sA0b5Zv7byVI/
for context and for discussion of these particular renames.

Commands run to generate this change:

gorename -from '"cmd/compile/internal/gc".OPROC' -to OGO
gorename -from '"cmd/compile/internal/gc".OCOM' -to OBITNOT
gorename -from '"cmd/compile/internal/gc".OMINUS' -to ONEG
gorename -from '"cmd/compile/internal/gc".OIND' -to ODEREF
gorename -from '"cmd/compile/internal/gc".OARRAYBYTESTR' -to OBYTES2STR
gorename -from '"cmd/compile/internal/gc".OARRAYBYTESTRTMP' -to OBYTES2STRTMP
gorename -from '"cmd/compile/internal/gc".OARRAYRUNESTR' -to ORUNES2STR
gorename -from '"cmd/compile/internal/gc".OSTRARRAYBYTE' -to OSTR2BYTES
gorename -from '"cmd/compile/internal/gc".OSTRARRAYBYTETMP' -to OSTR2BYTESTMP
gorename -from '"cmd/compile/internal/gc".OSTRARRAYRUNE' -to OSTR2RUNES

gorename -from '"cmd/compile/internal/gc".Etop' -to ctxStmt
gorename -from '"cmd/compile/internal/gc".Erv' -to ctxExpr
gorename -from '"cmd/compile/internal/gc".Ecall' -to ctxCallee
gorename -from '"cmd/compile/internal/gc".Efnstruct' -to ctxMultiOK
gorename -from '"cmd/compile/internal/gc".Easgn' -to ctxAssign
gorename -from '"cmd/compile/internal/gc".Ecomplit' -to ctxCompLit

Not altered: parameters and local variables (mostly in typecheck.go) named top,
which should probably now be called ctx (and which should probably have a named type).
Also not altered: Field called Top in gc.Func.

gorename -from '"cmd/compile/internal/gc".Node.Isddd' -to IsDDD
gorename -from '"cmd/compile/internal/gc".Node.SetIsddd' -to SetIsDDD
gorename -from '"cmd/compile/internal/gc".nodeIsddd' -to nodeIsDDD
gorename -from '"cmd/compile/internal/types".Field.Isddd' -to IsDDD
gorename -from '"cmd/compile/internal/types".Field.SetIsddd' -to SetIsDDD
gorename -from '"cmd/compile/internal/types".fieldIsddd' -to fieldIsDDD

Not altered: function gc.hasddd, params and local variables called isddd
Also not altered: fmt.go prints nodes using "isddd(%v)".

cd cmd/compile/internal/gc; go generate

I then manually found impacted comments using exact string match
and fixed them up by hand. The comment changes were trivial.

Passes toolstash-check.

Fixes #27167. If this experiment is deemed a success,
we will open a new tracking issue for renames to do
at the end of the 1.13 cycles.

Change-Id: I2dc541533d2ab0d06cb3d31d65df205ecfb151e8
Reviewed-on: https://go-review.googlesource.com/c/150140
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

josharian added a commit to josharian/go that referenced this issue Dec 30, 2018

cmd/compile: bulk rename
This change does a bulk rename of several identifiers in the compiler.
See golang#27167 and https://docs.google.com/document/d/19_ExiylD9MRfeAjKIfEsMU1_RGhuxB9sA0b5Zv7byVI/
for context and for discussion of these particular renames.

Commands run to generate this change:

gorename -from '"cmd/compile/internal/gc".OPROC' -to OGO
gorename -from '"cmd/compile/internal/gc".OCOM' -to OBITNOT
gorename -from '"cmd/compile/internal/gc".OMINUS' -to ONEG
gorename -from '"cmd/compile/internal/gc".OIND' -to ODEREF
gorename -from '"cmd/compile/internal/gc".OARRAYBYTESTR' -to OBYTES2STR
gorename -from '"cmd/compile/internal/gc".OARRAYBYTESTRTMP' -to OBYTES2STRTMP
gorename -from '"cmd/compile/internal/gc".OARRAYRUNESTR' -to ORUNES2STR
gorename -from '"cmd/compile/internal/gc".OSTRARRAYBYTE' -to OSTR2BYTES
gorename -from '"cmd/compile/internal/gc".OSTRARRAYBYTETMP' -to OSTR2BYTESTMP
gorename -from '"cmd/compile/internal/gc".OSTRARRAYRUNE' -to OSTR2RUNES

gorename -from '"cmd/compile/internal/gc".Etop' -to ctxStmt
gorename -from '"cmd/compile/internal/gc".Erv' -to ctxExpr
gorename -from '"cmd/compile/internal/gc".Ecall' -to ctxCallee
gorename -from '"cmd/compile/internal/gc".Efnstruct' -to ctxMultiOK
gorename -from '"cmd/compile/internal/gc".Easgn' -to ctxAssign
gorename -from '"cmd/compile/internal/gc".Ecomplit' -to ctxCompLit

Not altered: parameters and local variables (mostly in typecheck.go) named top,
which should probably now be called ctx (and which should probably have a named type).
Also not altered: Field called Top in gc.Func.

gorename -from '"cmd/compile/internal/gc".Node.Isddd' -to IsDDD
gorename -from '"cmd/compile/internal/gc".Node.SetIsddd' -to SetIsDDD
gorename -from '"cmd/compile/internal/gc".nodeIsddd' -to nodeIsDDD
gorename -from '"cmd/compile/internal/types".Field.Isddd' -to IsDDD
gorename -from '"cmd/compile/internal/types".Field.SetIsddd' -to SetIsDDD
gorename -from '"cmd/compile/internal/types".fieldIsddd' -to fieldIsDDD

Not altered: function gc.hasddd, params and local variables called isddd
Also not altered: fmt.go prints nodes using "isddd(%v)".

cd cmd/compile/internal/gc; go generate

I then manually found impacted comments using exact string match
and fixed them up by hand. The comment changes were trivial.

Passes toolstash-check.

Fixes golang#27167. If this experiment is deemed a success,
we will open a new tracking issue for renames to do
at the end of the 1.13 cycles.

Change-Id: I2dc541533d2ab0d06cb3d31d65df205ecfb151e8
@josharian

This comment has been minimized.

Copy link
Contributor Author

josharian commented Feb 26, 2019

This bulk rename went reasonably smoothly for Go 1.12. Let's do it again (assuming we find things we want to rename). Let's use this issue as a scratch pad to jot down things we'd like to rename for Go 1.13, and then near the end of the cycle we'll use a google doc again to discuss and agree on new names.

@josharian josharian reopened this Feb 26, 2019

@josharian josharian modified the milestones: Go1.12, Go1.13 Feb 26, 2019

@josharian josharian self-assigned this Feb 26, 2019

@josharian

This comment has been minimized.

Copy link
Contributor Author

josharian commented Mar 12, 2019

mayAffectMemory -> mightAffectMemory? (permission vs epistemic uncertainty)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.