• Josh Bleecher Snyder's avatar
    cmd/compile: eliminate fmtmode and fmtpkgpfx globals · 723ba180
    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: default avatarRobert Griesemer <gri@golang.org>
    723ba180
fmt_test.go 22.8 KB