Merge pull request #504 from matthyx:master

PiperOrigin-RevId: 258654826

6 files changed, 114 insertions, 6 deletions

diff --git a/tools/go_generics/defs.bzl b/tools/go_generics/defs.bzl
index 999fb3426..c5be52ecd 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..e9cc2c753 100644
--- a/tools/go_generics/generics.go
+++ b/tools/go_generics/generics.go
@@ -82,7 +82,12 @@
 // 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 +113,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 +237,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()