From 1cbe02913bfa05e29e3b353cc269730c3759d986 Mon Sep 17 00:00:00 2001 From: Roger Peppe Date: Thu, 29 Aug 2024 13:10:55 +0100 Subject: [PATCH] internal/astinternal: simpler internal API As suggested in a too-tardy review here (https://review.gerrithub.io/c/cue-lang/cue/+/1200204) the explicit append API is kinda verbose and the undo logic is hard to follow. By putting the buffer into the debugPrinter type, everything becomes a bit cleaner. We also make share the same wrapping logic between slices and structs, making the commonality clearer. While we're about it, deal with nil interfaces and nil concrete types in the same place, making the logic a little simpler again. Signed-off-by: Roger Peppe Change-Id: Iba81a933754eef172b59c6f71b535cf21948afd1 --- internal/astinternal/debug.go | 204 ++++++++++++++++++---------------- 1 file changed, 107 insertions(+), 97 deletions(-) diff --git a/internal/astinternal/debug.go b/internal/astinternal/debug.go index a32acf7aa..9be711c99 100644 --- a/internal/astinternal/debug.go +++ b/internal/astinternal/debug.go @@ -30,10 +30,14 @@ import ( // AppendDebug writes a multi-line Go-like representation of a syntax tree node, // including node position information and any relevant Go types. func AppendDebug(dst []byte, node ast.Node, config DebugConfig) []byte { - d := &debugPrinter{cfg: config} - dst = d.value(dst, reflect.ValueOf(node), nil) - dst = d.newline(dst) - return dst + d := &debugPrinter{ + cfg: config, + buf: dst, + } + if d.value(reflect.ValueOf(node), nil) { + d.newline() + } + return d.buf } // DebugConfig configures the behavior of [AppendDebug]. @@ -49,161 +53,167 @@ type DebugConfig struct { type debugPrinter struct { w io.Writer + buf []byte cfg DebugConfig level int } -func (d *debugPrinter) printf(dst []byte, format string, args ...any) []byte { - return fmt.Appendf(dst, format, args...) -} - -func (d *debugPrinter) newline(dst []byte) []byte { - return fmt.Appendf(dst, "\n%s", strings.Repeat("\t", d.level)) -} - var ( typeTokenPos = reflect.TypeFor[token.Pos]() typeTokenToken = reflect.TypeFor[token.Token]() ) -func (d *debugPrinter) value(dst []byte, v reflect.Value, impliedType reflect.Type) []byte { +// value produces the given value, omitting type information if +// its type is the same as implied type. It reports whether +// anything was produced. +func (d *debugPrinter) value(v reflect.Value, impliedType reflect.Type) bool { + start := d.pos() + d.value0(v, impliedType) + return d.pos() > start +} + +func (d *debugPrinter) value0(v reflect.Value, impliedType reflect.Type) { if d.cfg.Filter != nil && !d.cfg.Filter(v) { - return dst + return } // Skip over interface types. if v.Kind() == reflect.Interface { v = v.Elem() } - // Indirecting a nil interface gives a zero value. - if !v.IsValid() { + // Indirecting a nil interface gives a zero value. We + // can also have a nil pointer inside a non-nil interface. + if !v.IsValid() || (v.Kind() == reflect.Pointer && v.IsNil()) { if !d.cfg.OmitEmpty { - dst = d.printf(dst, "nil") + d.printf("nil") } - return dst + return } // We print the original pointer type if there was one. origType := v.Type() v = reflect.Indirect(v) - // Indirecting a nil pointer gives a zero value. - if !v.IsValid() { - if !d.cfg.OmitEmpty { - dst = d.printf(dst, "nil") - } - return dst - } if d.cfg.OmitEmpty && v.IsZero() { - return dst + return } t := v.Type() switch t { // Simple types which can stringify themselves. case typeTokenPos, typeTokenToken: - dst = d.printf(dst, "%s(%q)", t, v) + d.printf("%s(%q)", t, v) // Show relative positions too, if there are any, as they affect formatting. if t == typeTokenPos { pos := v.Interface().(token.Pos) if pos.HasRelPos() { - dst = d.printf(dst, ".WithRel(%q)", pos.RelPos()) + d.printf(".WithRel(%q)", pos.RelPos()) } } - return dst + return } - undoValue := len(dst) switch t.Kind() { default: // We assume all other kinds are basic in practice, like string or bool. if t.PkgPath() != "" { // Mention defined and non-predeclared types, for clarity. - dst = d.printf(dst, "%s(%#v)", t, v) + d.printf("%s(%#v)", t, v) } else { - dst = d.printf(dst, "%#v", v) + d.printf("%#v", v) } - case reflect.Slice: + case reflect.Slice, reflect.Struct: + valueStart := d.pos() if origType != impliedType { - dst = d.printf(dst, "%s", origType) + d.printf("%s", origType) } - dst = d.printf(dst, "{") + d.printf("{") d.level++ - anyElems := false - for i := 0; i < v.Len(); i++ { - ev := v.Index(i) - undoElem := len(dst) - dst = d.newline(dst) - // Note: a slice literal implies the type of its elements - // so we can avoid mentioning the type - // of each element if it matches. - if dst2 := d.value(dst, ev, t.Elem()); len(dst2) == len(dst) { - dst = dst[:undoElem] - } else { - dst = dst2 - anyElems = true - } + var anyElems bool + if t.Kind() == reflect.Slice { + anyElems = d.sliceElems(v, t.Elem()) + } else { + anyElems = d.structFields(v) } d.level-- if !anyElems && d.cfg.OmitEmpty { - dst = dst[:undoValue] + d.truncate(valueStart) } else { if anyElems { - dst = d.newline(dst) + d.newline() } - dst = d.printf(dst, "}") + d.printf("}") } + } +} - case reflect.Struct: - if origType != impliedType { - dst = d.printf(dst, "%s", origType) - } - dst = d.printf(dst, "{") - anyElems := false - d.level++ - for i := 0; i < v.NumField(); i++ { - f := t.Field(i) - if !gotoken.IsExported(f.Name) { - continue - } - switch f.Name { - // These fields are cyclic, and they don't represent the syntax anyway. - case "Scope", "Node", "Unresolved": - continue - } - undoElem := len(dst) - dst = d.newline(dst) - dst = d.printf(dst, "%s: ", f.Name) - if dst2 := d.value(dst, v.Field(i), nil); len(dst2) == len(dst) { - dst = dst[:undoElem] - } else { - dst = dst2 - anyElems = true - } - } - val := v.Addr().Interface() - if val, ok := val.(ast.Node); ok { - // Comments attached to a node aren't a regular field, but are still useful. - // The majority of nodes won't have comments, so skip them when empty. - if comments := ast.Comments(val); len(comments) > 0 { - anyElems = true - dst = d.newline(dst) - dst = d.printf(dst, "Comments: ") - dst = d.value(dst, reflect.ValueOf(comments), nil) - } +func (d *debugPrinter) sliceElems(v reflect.Value, elemType reflect.Type) (anyElems bool) { + for i := 0; i < v.Len(); i++ { + ev := v.Index(i) + elemStart := d.pos() + d.newline() + // Note: a slice literal implies the type of its elements + // so we can avoid mentioning the type + // of each element if it matches. + if d.value(ev, elemType) { + anyElems = true + } else { + d.truncate(elemStart) } - d.level-- - if !anyElems && d.cfg.OmitEmpty { - dst = dst[:undoValue] + } + return anyElems +} + +func (d *debugPrinter) structFields(v reflect.Value) (anyElems bool) { + t := v.Type() + for i := 0; i < v.NumField(); i++ { + f := t.Field(i) + if !gotoken.IsExported(f.Name) { + continue + } + switch f.Name { + // These fields are cyclic, and they don't represent the syntax anyway. + case "Scope", "Node", "Unresolved": + continue + } + elemStart := d.pos() + d.newline() + d.printf("%s: ", f.Name) + if d.value(v.Field(i), nil) { + anyElems = true } else { - if anyElems { - dst = d.newline(dst) - } - dst = d.printf(dst, "}") + d.truncate(elemStart) } } - return dst + val := v.Addr().Interface() + if val, ok := val.(ast.Node); ok { + // Comments attached to a node aren't a regular field, but are still useful. + // The majority of nodes won't have comments, so skip them when empty. + if comments := ast.Comments(val); len(comments) > 0 { + anyElems = true + d.newline() + d.printf("Comments: ") + d.value(reflect.ValueOf(comments), nil) + } + } + return anyElems +} + +func (d *debugPrinter) printf(format string, args ...any) { + d.buf = fmt.Appendf(d.buf, format, args...) +} + +func (d *debugPrinter) newline() { + d.buf = fmt.Appendf(d.buf, "\n%s", strings.Repeat("\t", d.level)) +} + +func (d *debugPrinter) pos() int { + return len(d.buf) +} + +func (d *debugPrinter) truncate(pos int) { + d.buf = d.buf[:pos] } func DebugStr(x interface{}) (out string) {