From 239d7c6fdf344fe7051328056b7578657c7b6950 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Thu, 4 Jul 2019 08:33:00 +0200 Subject: go_generics: treat the Sel part of an ast.SelectorExpr --- tools/go_generics/defs.bzl | 4 ++ tools/go_generics/generics.go | 9 ++++- tools/go_generics/generics_tests/anon/input.go | 46 ++++++++++++++++++++++ tools/go_generics/generics_tests/anon/opts.txt | 1 + .../generics_tests/anon/output/output.go | 42 ++++++++++++++++++++ tools/go_generics/globals/globals_visitor.go | 17 ++++++-- 6 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 tools/go_generics/generics_tests/anon/input.go create mode 100644 tools/go_generics/generics_tests/anon/opts.txt create mode 100644 tools/go_generics/generics_tests/anon/output/output.go diff --git a/tools/go_generics/defs.bzl b/tools/go_generics/defs.bzl index 999fb3426..596bf51be 100644 --- a/tools/go_generics/defs.bzl +++ b/tools/go_generics/defs.bzl @@ -93,6 +93,9 @@ def _go_template_instance_impl(ctx): args += [("-c=%s=%s" % (p[0], p[1])) for p in ctx.attr.consts.items()] args += [("-import=%s=%s" % (p[0], p[1])) for p in ctx.attr.imports.items()] + if ctx.attr.anon: + args += ["-anon"] + ctx.actions.run( inputs = [template.file], outputs = [output], @@ -129,6 +132,7 @@ go_template_instance = rule( "types": attr.string_dict(), "consts": attr.string_dict(), "imports": attr.string_dict(), + "anon": attr.bool(mandatory=False, default=False), "package": attr.string(mandatory = True), "out": attr.output(mandatory = True), "_tool": attr.label(executable = True, cfg = "host", default = Label("//tools/go_generics")), diff --git a/tools/go_generics/generics.go b/tools/go_generics/generics.go index 22c714c13..e3912ef2a 100644 --- a/tools/go_generics/generics.go +++ b/tools/go_generics/generics.go @@ -82,7 +82,11 @@ // Note that the second call to g() kept "b" as an argument because it refers to // the local variable "b". // -// Unfortunately, go_generics does not handle anonymous fields with renamed types. +// Note that go_generics can handle anonymous fields with renamed types if -anon is passed in, +// however it does not perform strict checking on parameter types that share the same name +// as the global type and therefore will rename them as well. +// +// You can see an example in the tools/go_generics/generics_tests/interface test. package main import ( @@ -108,6 +112,7 @@ var ( prefix = flag.String("prefix", "", "`prefix` to add to each global symbol") packageName = flag.String("p", "main", "output package `name`") printAST = flag.Bool("ast", false, "prints the AST") + processAnon = flag.Bool("anon", false, "process anonymous fields") types = make(mapValue) consts = make(mapValue) imports = make(mapValue) @@ -231,7 +236,7 @@ func main() { } } } - }) + }, *processAnon) // Remove the definition of all types that are being remapped. set := make(typeSet) diff --git a/tools/go_generics/generics_tests/anon/input.go b/tools/go_generics/generics_tests/anon/input.go new file mode 100644 index 000000000..44086d522 --- /dev/null +++ b/tools/go_generics/generics_tests/anon/input.go @@ -0,0 +1,46 @@ +// Copyright 2019 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package tests + +type T interface { + Apply(T) T +} + +type Foo struct { + T + Bar map[string]T `json:"bar,omitempty"` +} + +type Baz struct { + T someTypeNotT +} + +func (f Foo) GetBar(name string) T { + b, ok := f.Bar[name] + if ok { + b = f.Apply(b) + } else { + b = f.T + } + return b +} + +func foobar() { + a := Baz{} + a.T = 0 // should not be renamed, this is a limitation + + b := otherpkg.UnrelatedType{} + b.T = 0 // should not be renamed, this is a limitation +} diff --git a/tools/go_generics/generics_tests/anon/opts.txt b/tools/go_generics/generics_tests/anon/opts.txt new file mode 100644 index 000000000..a5e9d26de --- /dev/null +++ b/tools/go_generics/generics_tests/anon/opts.txt @@ -0,0 +1 @@ +-t=T=Q -suffix=New -anon diff --git a/tools/go_generics/generics_tests/anon/output/output.go b/tools/go_generics/generics_tests/anon/output/output.go new file mode 100644 index 000000000..160cddf79 --- /dev/null +++ b/tools/go_generics/generics_tests/anon/output/output.go @@ -0,0 +1,42 @@ +// Copyright 2019 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +type FooNew struct { + Q + Bar map[string]Q `json:"bar,omitempty"` +} + +type BazNew struct { + T someTypeNotT +} + +func (f FooNew) GetBar(name string) Q { + b, ok := f.Bar[name] + if ok { + b = f.Apply(b) + } else { + b = f.Q + } + return b +} + +func foobarNew() { + a := BazNew{} + a.Q = 0 // should not be renamed, this is a limitation + + b := otherpkg.UnrelatedType{} + b.Q = 0 // should not be renamed, this is a limitation +} diff --git a/tools/go_generics/globals/globals_visitor.go b/tools/go_generics/globals/globals_visitor.go index 3f948637b..883f21ebe 100644 --- a/tools/go_generics/globals/globals_visitor.go +++ b/tools/go_generics/globals/globals_visitor.go @@ -47,6 +47,11 @@ type globalsVisitor struct { // scope is the current scope as nodes are visited. scope *scope + + // processAnon indicates whether we should process anonymous struct fields. + // It does not perform strict checking on parameter types that share the same name + // as the global type and therefore will rename them as well. + processAnon bool } // unexpected is called when an unexpected node appears in the AST. It dumps @@ -307,6 +312,9 @@ func (v *globalsVisitor) visitExpr(ge ast.Expr) { case *ast.SelectorExpr: v.visitExpr(e.X) + if v.processAnon { + v.visitExpr(e.Sel) + } case *ast.SliceExpr: v.visitExpr(e.X) @@ -577,11 +585,12 @@ func (v *globalsVisitor) visit() { // // The function f() is allowed to modify the identifier, for example, to rename // uses of global references. -func Visit(fset *token.FileSet, file *ast.File, f func(*ast.Ident, SymKind)) { +func Visit(fset *token.FileSet, file *ast.File, f func(*ast.Ident, SymKind), processAnon bool) { v := globalsVisitor{ - fset: fset, - file: file, - f: f, + fset: fset, + file: file, + f: f, + processAnon: processAnon, } v.visit() -- cgit v1.2.3