summaryrefslogtreecommitdiffhomepage
path: root/tools/go_marshal
diff options
context:
space:
mode:
Diffstat (limited to 'tools/go_marshal')
-rw-r--r--tools/go_marshal/README.md15
-rw-r--r--tools/go_marshal/defs.bzl9
-rw-r--r--tools/go_marshal/gomarshal/generator.go85
-rw-r--r--tools/go_marshal/gomarshal/generator_interfaces.go4
-rw-r--r--tools/go_marshal/gomarshal/generator_interfaces_struct.go15
-rw-r--r--tools/go_marshal/gomarshal/util.go20
-rw-r--r--tools/go_marshal/main.go11
-rw-r--r--tools/go_marshal/test/test.go24
8 files changed, 131 insertions, 52 deletions
diff --git a/tools/go_marshal/README.md b/tools/go_marshal/README.md
index 75e5c7888..d8045c295 100644
--- a/tools/go_marshal/README.md
+++ b/tools/go_marshal/README.md
@@ -113,3 +113,18 @@ The following are some guidelines for modifying the `go_marshal` tool:
- No runtime reflection in the code generated for the marshallable interface.
The entire point of the tool is to avoid runtime reflection. The generated
tests may use reflection.
+
+## Debugging
+
+To enable debugging output from the go-marshal tool, use one of the following
+options, depending on how go-marshal is being invoked:
+
+- Pass `--define gomarshal=verbose` to the bazel command. Note that this can
+ generate a lot of output depending on what's being compiled, as this will
+ enable debugging for all packages built by the command.
+
+- Set `marshal_debug = True` on the top-level `go_library` BUILD rule.
+
+- Set `debug = True` on the `go_marshal` BUILD rule.
+
+- Pass `-debug` to the go-marshal tool invocation.
diff --git a/tools/go_marshal/defs.bzl b/tools/go_marshal/defs.bzl
index ba98f3599..f44f83eab 100644
--- a/tools/go_marshal/defs.bzl
+++ b/tools/go_marshal/defs.bzl
@@ -4,11 +4,13 @@ def _go_marshal_impl(ctx):
"""Execute the go_marshal tool."""
output = ctx.outputs.lib
output_test = ctx.outputs.test
+ output_test_unconditional = ctx.outputs.test_unconditional
# Run the marshal command.
args = ["-output=%s" % output.path]
- args += ["-pkg=%s" % ctx.attr.package]
- args += ["-output_test=%s" % output_test.path]
+ args.append("-pkg=%s" % ctx.attr.package)
+ args.append("-output_test=%s" % output_test.path)
+ args.append("-output_test_unconditional=%s" % output_test_unconditional.path)
if ctx.attr.debug:
args += ["-debug"]
@@ -18,7 +20,7 @@ def _go_marshal_impl(ctx):
args += [f.path for f in src.files.to_list()]
ctx.actions.run(
inputs = ctx.files.srcs,
- outputs = [output, output_test],
+ outputs = [output, output_test, output_test_unconditional],
mnemonic = "GoMarshal",
progress_message = "go_marshal: %s" % ctx.label,
arguments = args,
@@ -48,6 +50,7 @@ go_marshal = rule(
outputs = {
"lib": "%{name}_unsafe.go",
"test": "%{name}_test.go",
+ "test_unconditional": "%{name}_unconditional_test.go",
},
)
diff --git a/tools/go_marshal/gomarshal/generator.go b/tools/go_marshal/gomarshal/generator.go
index 72ed6d109..4a53d25be 100644
--- a/tools/go_marshal/gomarshal/generator.go
+++ b/tools/go_marshal/gomarshal/generator.go
@@ -68,6 +68,8 @@ type Generator struct {
output *os.File
// Output file to write generated tests.
outputTest *os.File
+ // Output file to write unconditionally generated tests.
+ outputTestUC *os.File
// Package name for the generated file.
pkg string
// Set of extra packages to import in the generated file.
@@ -75,21 +77,26 @@ type Generator struct {
}
// NewGenerator creates a new code Generator.
-func NewGenerator(srcs []string, out, outTest, pkg string, imports []string) (*Generator, error) {
+func NewGenerator(srcs []string, out, outTest, outTestUnconditional, pkg string, imports []string) (*Generator, error) {
f, err := os.OpenFile(out, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644)
if err != nil {
- return nil, fmt.Errorf("Couldn't open output file %q: %v", out, err)
+ return nil, fmt.Errorf("couldn't open output file %q: %w", out, err)
}
fTest, err := os.OpenFile(outTest, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644)
if err != nil {
- return nil, fmt.Errorf("Couldn't open test output file %q: %v", out, err)
+ return nil, fmt.Errorf("couldn't open test output file %q: %w", out, err)
+ }
+ fTestUC, err := os.OpenFile(outTestUnconditional, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644)
+ if err != nil {
+ return nil, fmt.Errorf("couldn't open unconditional test output file %q: %w", out, err)
}
g := Generator{
- inputs: srcs,
- output: f,
- outputTest: fTest,
- pkg: pkg,
- imports: newImportTable(),
+ inputs: srcs,
+ output: f,
+ outputTest: fTest,
+ outputTestUC: fTestUC,
+ pkg: pkg,
+ imports: newImportTable(),
}
for _, i := range imports {
// All imports on the extra imports list are unconditionally marked as
@@ -174,7 +181,7 @@ func (g *Generator) parse() ([]*ast.File, []*token.FileSet, error) {
f, err := parser.ParseFile(fset, path, nil, parser.ParseComments)
if err != nil {
// Not a valid input file?
- return nil, nil, fmt.Errorf("Input %q can't be parsed: %v", path, err)
+ return nil, nil, fmt.Errorf("input %q can't be parsed: %w", path, err)
}
if debugEnabled() {
@@ -454,6 +461,46 @@ func (g *Generator) Run() error {
// source file.
func (g *Generator) writeTests(ts []*testGenerator) error {
var b sourceBuffer
+
+ // Write the unconditional test file. This file is always compiled,
+ // regardless of what build tags were specified on the original input
+ // files. We use this file to guarantee we never end up with an empty test
+ // file, as that causes the build to fail with "no tests/benchmarks/examples
+ // found".
+ //
+ // There's no easy way to determine ahead of time if we'll end up with an
+ // empty build file since build constraints can arbitrarily cause some of
+ // the original types to be not defined. We also have no way to tell bazel
+ // to omit the entire test suite since the output files are already defined
+ // before go-marshal is called.
+ b.emit("// Automatically generated marshal tests. See tools/go_marshal.\n\n")
+ b.emit("package %s\n\n", g.pkg)
+ b.emit("func Example() {\n")
+ b.inIndent(func() {
+ b.emit("// This example is intentionally empty, and ensures this package contains at\n")
+ b.emit("// least one testable entity. go-marshal is forced to emit a test package if the\n")
+ b.emit("// input package is marked marshallable, but emitting no testable entities \n")
+ b.emit("// results in a build failure.\n")
+ })
+ b.emit("}\n")
+ if err := b.write(g.outputTestUC); err != nil {
+ return err
+ }
+
+ // Now generate the real test file that contains the real types we
+ // processed. These need to be conditionally compiled according to the build
+ // tags, as the original types may not be defined under all build
+ // configurations.
+
+ b.reset()
+ b.emit("// Automatically generated marshal tests. See tools/go_marshal.\n\n")
+
+ // Emit build tags.
+ if t := tags.Aggregate(g.inputs); len(t) > 0 {
+ b.emit(strings.Join(t.Lines(), "\n"))
+ b.emit("\n\n")
+ }
+
b.emit("package %s\n\n", g.pkg)
if err := b.write(g.outputTest); err != nil {
return err
@@ -470,26 +517,6 @@ func (g *Generator) writeTests(ts []*testGenerator) error {
}
// Write test functions.
-
- // If we didn't generate any Marshallable implementations, we can't just
- // emit an empty test file, since that causes the build to fail with "no
- // tests/benchmarks/examples found". Unfortunately we can't signal bazel to
- // omit the entire package since the outputs are already defined before
- // go-marshal is called. If we'd otherwise emit an empty test suite, emit an
- // empty example instead.
- if len(ts) == 0 {
- b.reset()
- b.emit("func Example() {\n")
- b.inIndent(func() {
- b.emit("// This example is intentionally empty to ensure this file contains at least\n")
- b.emit("// one testable entity. go-marshal is forced to emit a test file if a package\n")
- b.emit("// is marked marshallable, but emitting a test file with no entities results\n")
- b.emit("// in a build failure.\n")
- })
- b.emit("}\n")
- return b.write(g.outputTest)
- }
-
for _, t := range ts {
if err := t.write(g.outputTest); err != nil {
return err
diff --git a/tools/go_marshal/gomarshal/generator_interfaces.go b/tools/go_marshal/gomarshal/generator_interfaces.go
index cf76b5241..36447b86b 100644
--- a/tools/go_marshal/gomarshal/generator_interfaces.go
+++ b/tools/go_marshal/gomarshal/generator_interfaces.go
@@ -43,8 +43,8 @@ type interfaceGenerator struct {
// of t's interfaces.
ms map[string]struct{}
- // as records embedded fields in t that are potentially not packed. The key
- // is the accessor for the field.
+ // as records fields in t that are potentially not packed. The key is the
+ // accessor for the field.
as map[string]struct{}
}
diff --git a/tools/go_marshal/gomarshal/generator_interfaces_struct.go b/tools/go_marshal/gomarshal/generator_interfaces_struct.go
index d3fc1c1c6..fe76d3785 100644
--- a/tools/go_marshal/gomarshal/generator_interfaces_struct.go
+++ b/tools/go_marshal/gomarshal/generator_interfaces_struct.go
@@ -20,6 +20,7 @@ package gomarshal
import (
"fmt"
"go/ast"
+ "sort"
"strings"
)
@@ -40,6 +41,8 @@ func (g *interfaceGenerator) areFieldsPackedExpression() (string, bool) {
for accessor, _ := range g.as {
cs = append(cs, fmt.Sprintf("%s.Packed()", accessor))
}
+ // Sort expressions for determinstic build outputs.
+ sort.Strings(cs)
return strings.Join(cs, " && "), true
}
@@ -48,12 +51,6 @@ func (g *interfaceGenerator) areFieldsPackedExpression() (string, bool) {
// later.
func (g *interfaceGenerator) validateStruct(ts *ast.TypeSpec, st *ast.StructType) {
forEachStructField(st, func(f *ast.Field) {
- if len(f.Names) == 0 {
- g.abortAt(f.Pos(), "Cannot marshal structs with embedded fields, give the field a name; use '_' for anonymous fields such as padding fields")
- }
- })
-
- forEachStructField(st, func(f *ast.Field) {
fieldDispatcher{
primitive: func(_, t *ast.Ident) {
g.validatePrimitiveNewtype(t)
@@ -98,7 +95,7 @@ func (g *interfaceGenerator) emitMarshallableForStruct(st *ast.StructType) {
var dynamicSizeTerms []string
forEachStructField(st, fieldDispatcher{
- primitive: func(n, t *ast.Ident) {
+ primitive: func(_, t *ast.Ident) {
if size, dynamic := g.scalarSize(t); !dynamic {
primitiveSize += size
} else {
@@ -106,13 +103,13 @@ func (g *interfaceGenerator) emitMarshallableForStruct(st *ast.StructType) {
dynamicSizeTerms = append(dynamicSizeTerms, fmt.Sprintf("(*%s)(nil).SizeBytes()", t.Name))
}
},
- selector: func(n, tX, tSel *ast.Ident) {
+ selector: func(_, tX, tSel *ast.Ident) {
tName := fmt.Sprintf("%s.%s", tX.Name, tSel.Name)
g.recordUsedImport(tX.Name)
g.recordUsedMarshallable(tName)
dynamicSizeTerms = append(dynamicSizeTerms, fmt.Sprintf("(*%s)(nil).SizeBytes()", tName))
},
- array: func(n *ast.Ident, a *ast.ArrayType, t *ast.Ident) {
+ array: func(_ *ast.Ident, a *ast.ArrayType, t *ast.Ident) {
lenExpr := g.arrayLenExpr(a)
if size, dynamic := g.scalarSize(t); !dynamic {
dynamicSizeTerms = append(dynamicSizeTerms, fmt.Sprintf("%d*%s", size, lenExpr))
diff --git a/tools/go_marshal/gomarshal/util.go b/tools/go_marshal/gomarshal/util.go
index d94314302..6a42691cd 100644
--- a/tools/go_marshal/gomarshal/util.go
+++ b/tools/go_marshal/gomarshal/util.go
@@ -79,7 +79,7 @@ type fieldDispatcher struct {
}
// Precondition: All dispatch callbacks that will be invoked must be
-// provided. Embedded fields are not allowed, len(f.Names) >= 1.
+// provided.
func (fd fieldDispatcher) dispatch(f *ast.Field) {
// Each field declaration may actually be multiple declarations of the same
// type. For example, consider:
@@ -88,12 +88,24 @@ func (fd fieldDispatcher) dispatch(f *ast.Field) {
// x, y, z int
// }
//
- // We invoke the call-backs once per such instance. Embedded fields are not
- // allowed, and results in a panic.
+ // We invoke the call-backs once per such instance.
+
+ // Handle embedded fields. Embedded fields have no names, but can be
+ // referenced by the type name.
if len(f.Names) < 1 {
- panic("Precondition not met: attempted to dispatch on embedded field")
+ switch v := f.Type.(type) {
+ case *ast.Ident:
+ fd.primitive(v, v)
+ case *ast.SelectorExpr:
+ fd.selector(v.Sel, v.X.(*ast.Ident), v.Sel)
+ default:
+ // Note: Arrays can't be embedded, which is handled here.
+ panic(fmt.Sprintf("Attempted to dispatch on embedded field of unsupported kind: %#v", f.Type))
+ }
+ return
}
+ // Non-embedded field.
for _, name := range f.Names {
switch v := f.Type.(type) {
case *ast.Ident:
diff --git a/tools/go_marshal/main.go b/tools/go_marshal/main.go
index f74be5c29..6e4a3e8c4 100644
--- a/tools/go_marshal/main.go
+++ b/tools/go_marshal/main.go
@@ -31,10 +31,11 @@ import (
)
var (
- pkg = flag.String("pkg", "", "output package")
- output = flag.String("output", "", "output file")
- outputTest = flag.String("output_test", "", "output file for tests")
- imports = flag.String("imports", "", "comma-separated list of extra packages to import in generated code")
+ pkg = flag.String("pkg", "", "output package")
+ output = flag.String("output", "", "output file")
+ outputTest = flag.String("output_test", "", "output file for tests")
+ outputTestUnconditional = flag.String("output_test_unconditional", "", "output file for unconditional tests")
+ imports = flag.String("imports", "", "comma-separated list of extra packages to import in generated code")
)
func main() {
@@ -61,7 +62,7 @@ func main() {
// as an import.
extraImports = strings.Split(*imports, ",")
}
- g, err := gomarshal.NewGenerator(flag.Args(), *output, *outputTest, *pkg, extraImports)
+ g, err := gomarshal.NewGenerator(flag.Args(), *output, *outputTest, *outputTestUnconditional, *pkg, extraImports)
if err != nil {
panic(err)
}
diff --git a/tools/go_marshal/test/test.go b/tools/go_marshal/test/test.go
index f75ca1b7f..d9e9f341b 100644
--- a/tools/go_marshal/test/test.go
+++ b/tools/go_marshal/test/test.go
@@ -174,3 +174,27 @@ type Type9 struct {
x int64
y [sizeA]int32
}
+
+// Type10Embed is a test data type which is be embedded into another type.
+//
+// +marshal
+type Type10Embed struct {
+ x int64
+}
+
+// Type10 is a test data type which contains an embedded struct.
+//
+// +marshal
+type Type10 struct {
+ Type10Embed
+ y int64
+}
+
+// Type11 is a test data type which contains an embedded struct from an external
+// package.
+//
+// +marshal
+type Type11 struct {
+ ex.External
+ y int64
+}
generated by cgit v1.2.3 (git 2.39.1) at 2024-12-29 03:54:05 +0000