golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
120.1k stars 17.24k forks source link

proposal: debug/bisect: publish x/tools/internal/bisect #67140

Open rsc opened 2 weeks ago

rsc commented 2 weeks ago

golang.org/x/tools/cmd/bisect is incredibly useful for binary-searching over potential bug locations when a compiler or library change causes a problem. There is a small package that handles the protocol of speaking to the bisect tool, and we have two copies of it: std's internal/bisect (for the compiler and runtime) and also golang.org/x/tools/internal/bisect (for x/tools/cmd/bisect).

We have been using bisect successfully long enough that I feel pretty confident in the API. I propose we publish the package as the standard library package debug/bisect. This will let the compiler and runtime both use it, it will let us drop down to having just one copy of the code, and it will also make the logic available to other programs that want to be bisect-debuggable. In particular, packages beyond the standard library could use it to provide bisectable implementation changes when they make compatible-but-possibly-breaking changes.

The full API is:

package bisect

func AppendMarker(dst []byte, id uint64) []byte
func CutMarker(line string) (short string, id uint64, ok bool)
func Hash(data ...any) uint64
func Marker(id uint64) string
func PrintMarker(w Writer, h uint64) error

type Matcher struct { ... }
func New(pattern string) (*Matcher, error)
func (*Matcher) FileLine(w Writer, file string, line int) bool
func (*Matcher) MarkerOnly() bool
func (*Matcher) ShouldEnable(id uint64) bool
func (*Matcher) ShouldPrint(id uint64) bool
func (*Matcher) Stack(w io.Writer) bool
aclements commented 1 day ago

Because of its very low-level uses by the runtime, package bisect has no imports, so it defines its own copy of io.Writer.

Alternatively, we keep most of the implementation in internal/bisect, and the public package is just a thin wrapper around this. In general, we're having to move more toward this model anyway for things that are used from runtime.

rsc commented 1 day ago

Thanks for pointing that out. Changed the API for the public package to use io.

rsc commented 1 day ago

This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group

hherman1 commented 1 day ago

I’m having a hard time understanding what this does, or how the api methods are meant to be used. Comments on the api methods would be helpful for me.