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/link: make code more readable #16818

Open
matloob opened this Issue Aug 21, 2016 · 5 comments

Comments

Projects
None yet
4 participants
@matloob
Contributor

matloob commented Aug 21, 2016

I'd like to clean up cmd/link and help improve its Go style.

A few things I'd like to do:

  • Remove a bunch of the global variables, especially Ctxt
  • Reverse the dependency between the ld package and the architecture specific packages.
  • Localize more of the state to smaller components

@matloob matloob self-assigned this Aug 21, 2016

@gopherbot

This comment has been minimized.

gopherbot commented Aug 21, 2016

CL https://golang.org/cl/27459 mentions this issue.

gopherbot pushed a commit that referenced this issue Aug 21, 2016

cmd/link/internal: remove global Ctxt variable
This change threads the *ld.Link Ctxt variable through
code in arch-specific packages. This removes all remaining
uses of Ctxt, so remove the global variable too.

This CL continues the work in golang.org/cl/27408

Updates #16818

Change-Id: I5f4536847a1825fd0b944824e8ae4e122ec0fb78
Reviewed-on: https://go-review.googlesource.com/27459
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

gopherbot pushed a commit that referenced this issue Aug 21, 2016

cmd/link: remove global Bso variable
Bso is already a member on ld.Link. Use that instead of
the global.

Updates #16818

Change-Id: Icfc0f6cb1ff551e8129253fb6b5e0d6a94479f51
Reviewed-on: https://go-review.googlesource.com/27470
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot

This comment has been minimized.

gopherbot commented Aug 21, 2016

CL https://golang.org/cl/27472 mentions this issue.

@gopherbot

This comment has been minimized.

gopherbot commented Aug 21, 2016

CL https://golang.org/cl/27471 mentions this issue.

gopherbot pushed a commit that referenced this issue Aug 21, 2016

cmd/link: use standard library flag package where possible
The obj library's flag functions are (mostly) light wrappers
around the standard library flag package. Use the flag package
directly where possible.

Most uses of the 'count'-type flags (except for -v) only check
against 0, so they can safely be replaced by bools. Only -v
and the flagfns haven't been replaced.

Debug has been turned into a slice of bools rather than ints.
There was a copy of the -v verbosity in ctxt.Debugvlog, so don't use
Debug['v'] and just use ctxt.Debugvlog.

Updates #16818

Change-Id: Icf6473a4823c9d35513bbd0c34ea02d5676d782a
Reviewed-on: https://go-review.googlesource.com/27471
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

gopherbot pushed a commit that referenced this issue Aug 21, 2016

cmd/link/internal/ld: rename pobj.go to main.go
The only thing pobj contains is the Ldmain function.

Updates #16818

Change-Id: Id114bdb264cb5ea2f372eb2166201f1f8eb99445
Reviewed-on: https://go-review.googlesource.com/27472
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot

This comment has been minimized.

gopherbot commented Aug 22, 2016

CL https://golang.org/cl/27473 mentions this issue.

gopherbot pushed a commit that referenced this issue Aug 22, 2016

cmd/link: turn some globals into flag pointer variables
This moves many of the flag globals into main and assigns them
to their flag.String/Int64/... directly.

Updates #16818

Change-Id: Ibbff44a273bbc5cb7228e43f147900ee8848517f
Reviewed-on: https://go-review.googlesource.com/27473
Reviewed-by: David Crawshaw <crawshaw@golang.org>
@gopherbot

This comment has been minimized.

gopherbot commented Aug 22, 2016

CL https://golang.org/cl/27468 mentions this issue.

gopherbot pushed a commit that referenced this issue Aug 23, 2016

cmd/link/internal/ld: camelCase a buch of snake_case names
I've also unexported a few symbols that weren't used outside the
package.

Updates #16818

Change-Id: I39d9d87b3eec30b88b4a17c1333cfbbfa6b3518f
Reviewed-on: https://go-review.googlesource.com/27468
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>

@quentinmit quentinmit added this to the Go1.8 milestone Sep 6, 2016

@quentinmit quentinmit added the NeedsFix label Oct 7, 2016

@rsc rsc modified the milestones: Unplanned, Go1.8 Oct 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment