-
Josh Bleecher Snyder authored
The fmtmode and fmtpkgpfx globals stand in the way of making the compiler more concurrent (#15756). This CL removes them. The natural way to eliminate a global is to explicitly thread it as a parameter through all function calls. However, most of the functions in gc/fmt.go get called indirectly, by way of fmt format strings, so there's nowhere natural to add a parameter. Since there are only a few fmtmode modes, use named types to distinguish between modes. For example, fmtNodeErr, fmtNodeDbg, and fmtNodeTypeId are all gc.Node, but they print in different modes. Varying the type allows us to thread mode through fmt. Handle fmtpkgpfx by converting it to a printing mode, FTypeIdName, and using the same type-based approach. To avoid a loss of readability and danger of bugs from introducing conversions at all call sites, instead add a helper that systematically modifies the args. The only remaining gc/fmt.go global is dumpdepth. Since that is used for debugging only, it that can be handled with a global mutex, or some similarly basic, if inefficient, protection. Passes toolstash -cmp. No compiler performance impact. For future reference, other options for threading state that were considered and rejected: * Wrapping values in structs, such as: type fmtNode struct { n *Node mode fmtMode } This reduces the proliferation of types, and supports easily adding extra local parameters. However, putting such a struct into an interface{} allocates. This is unacceptable in this particular area of code. * Passing state via precision, such as: fmt.Fprintf("%*v", mode, n) where mode is the state encoded as an integer. This avoids extra allocations, but it is out of keeping with the intended semantics of precision, and is less readable. * Modify the fmt package to support setting/getting context via fmt.State. Unavailable due to Go 1 compatibility, and probably the wrong solution anyway. * Give up on package fmt. This would be a huge readability regression and cause high code churn. * Attempt a de-novo rewrite that circumvents these problems. Too high a risk of bugs, with insufficient reward for the effort, particularly since long term plans call for elimination of gc.Node. Change-Id: Iea2440d5a34a938e64273707de27e3a897cb41d1 Reviewed-on: https://go-review.googlesource.com/38147 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
723ba180