From 5d863f89fed8f0580294ada88f92f72f361c598f Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 24 Aug 2021 14:30:24 -0700 Subject: cmd/compile: simplify bad conversion check Now that we're using OCONVIDATA(x) everywhere we formerly used OIDATA(OCONVIFACE(x)), there should be no OCONVIFACE operations that take a shape type. Change-Id: I4fb056456c60481c6dfe7bc111fca6223567e6a8 Reviewed-on: https://go-review.googlesource.com/c/go/+/344577 Trust: Keith Randall Trust: Dan Scales Run-TryBot: Keith Randall TryBot-Result: Go Bot Reviewed-by: Dan Scales --- src/cmd/compile/internal/noder/stencil.go | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/cmd/compile/internal/noder/stencil.go b/src/cmd/compile/internal/noder/stencil.go index 602e88c102..18a0506036 100644 --- a/src/cmd/compile/internal/noder/stencil.go +++ b/src/cmd/compile/internal/noder/stencil.go @@ -732,21 +732,15 @@ func (g *irgen) genericSubst(newsym *types.Sym, nameNode *ir.Name, shapes []*typ g.instTypeList = append(g.instTypeList, subst.ts.InstTypeList...) if doubleCheck { - okConvs := map[ir.Node]bool{} ir.Visit(newf, func(n ir.Node) { - if n.Op() == ir.OIDATA { - // IDATA(OCONVIFACE(x)) is ok, as we don't use the type of x. - // TODO: use some other op besides OCONVIFACE. ONEW might work - // (with appropriate direct vs. indirect interface cases). - okConvs[n.(*ir.UnaryExpr).X] = true + if n.Op() != ir.OCONVIFACE { + return } - if n.Op() == ir.OCONVIFACE && !okConvs[n] { - c := n.(*ir.ConvExpr) - if c.X.Type().HasShape() { - ir.Dump("BAD FUNCTION", newf) - ir.Dump("BAD CONVERSION", c) - base.Fatalf("converting shape type to interface") - } + c := n.(*ir.ConvExpr) + if c.X.Type().HasShape() { + ir.Dump("BAD FUNCTION", newf) + ir.Dump("BAD CONVERSION", c) + base.Fatalf("converting shape type to interface") } }) } -- cgit v1.2.3-54-g00ecf From 54cdef1f101a7a15fa6412fbedf8b009a1f725a1 Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Mon, 17 May 2021 12:31:11 -0700 Subject: reflect: add MapIter.SetKey and MapIter.SetValue These augment the existing MapIter.Key and MapIter.Value methods. The existing methods return new Values. Constructing these new Values often requires allocating. These methods allow the caller to bring their own storage. The naming is somewhat unfortunate, in that the spec uses the word "element" instead of "value", as do the reflect.Type methods. In a vacuum, MapIter.SetElem would be preferable. However, matching the existing methods is more important. Fixes #32424 Fixes #46131 Change-Id: I19c4d95c432f63dfe52cde96d2125abd021f24fa Reviewed-on: https://go-review.googlesource.com/c/go/+/320929 Trust: Josh Bleecher Snyder Run-TryBot: Josh Bleecher Snyder Reviewed-by: Keith Randall --- src/reflect/all_test.go | 41 ++++++++++++++++++++++++++++++++ src/reflect/value.go | 62 +++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index df79f05807..40ac6a95fa 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -336,6 +336,47 @@ func TestSetValue(t *testing.T) { } } +func TestMapIterSet(t *testing.T) { + m := make(map[string]interface{}, len(valueTests)) + for _, tt := range valueTests { + m[tt.s] = tt.i + } + v := ValueOf(m) + + k := New(v.Type().Key()).Elem() + e := New(v.Type().Elem()).Elem() + + iter := v.MapRange() + for iter.Next() { + iter.SetKey(k) + iter.SetValue(e) + want := m[k.String()] + got := e.Interface() + if got != want { + t.Errorf("%q: want (%T) %v, got (%T) %v", k.String(), want, want, got, got) + } + if setkey, key := valueToString(k), valueToString(iter.Key()); setkey != key { + t.Errorf("MapIter.Key() = %q, MapIter.SetKey() = %q", key, setkey) + } + if setval, val := valueToString(e), valueToString(iter.Value()); setval != val { + t.Errorf("MapIter.Value() = %q, MapIter.SetValue() = %q", val, setval) + } + } + + got := int(testing.AllocsPerRun(10, func() { + iter := v.MapRange() + for iter.Next() { + iter.SetKey(k) + iter.SetValue(e) + } + })) + // Making a *MapIter and making an hiter both allocate. + // Those should be the only two allocations. + if got != 2 { + t.Errorf("wanted 2 allocs, got %d", got) + } +} + func TestCanSetField(t *testing.T) { type embed struct{ x, X int } type Embed struct{ x, X int } diff --git a/src/reflect/value.go b/src/reflect/value.go index de01f13825..a8274cc871 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -1578,13 +1578,40 @@ func (it *MapIter) Key() Value { if it.it == nil { panic("MapIter.Key called before Next") } - if mapiterkey(it.it) == nil { + iterkey := mapiterkey(it.it) + if iterkey == nil { panic("MapIter.Key called on exhausted iterator") } t := (*mapType)(unsafe.Pointer(it.m.typ)) ktype := t.key - return copyVal(ktype, it.m.flag.ro()|flag(ktype.Kind()), mapiterkey(it.it)) + return copyVal(ktype, it.m.flag.ro()|flag(ktype.Kind()), iterkey) +} + +// SetKey assigns dst to the key of the iterator's current map entry. +// It is equivalent to dst.Set(it.Key()), but it avoids allocating a new Value. +// As in Go, the key must be assignable to dst's type. +func (it *MapIter) SetKey(dst Value) { + if it.it == nil { + panic("MapIter.SetKey called before Next") + } + iterkey := mapiterkey(it.it) + if iterkey == nil { + panic("MapIter.SetKey called on exhausted iterator") + } + + dst.mustBeAssignable() + var target unsafe.Pointer + if dst.kind() == Interface { + target = dst.ptr + } + + t := (*mapType)(unsafe.Pointer(it.m.typ)) + ktype := t.key + + key := Value{ktype, iterkey, it.m.flag | flag(ktype.Kind())} + key = key.assignTo("reflect.MapIter.SetKey", dst.typ, target) + typedmemmove(dst.typ, dst.ptr, key.ptr) } // Value returns the value of the iterator's current map entry. @@ -1592,13 +1619,40 @@ func (it *MapIter) Value() Value { if it.it == nil { panic("MapIter.Value called before Next") } - if mapiterkey(it.it) == nil { + iterelem := mapiterelem(it.it) + if iterelem == nil { panic("MapIter.Value called on exhausted iterator") } t := (*mapType)(unsafe.Pointer(it.m.typ)) vtype := t.elem - return copyVal(vtype, it.m.flag.ro()|flag(vtype.Kind()), mapiterelem(it.it)) + return copyVal(vtype, it.m.flag.ro()|flag(vtype.Kind()), iterelem) +} + +// SetValue assigns dst to the value of the iterator's current map entry. +// It is equivalent to dst.Set(it.Value()), but it avoids allocating a new Value. +// As in Go, the value must be assignable to dst's type. +func (it *MapIter) SetValue(dst Value) { + if it.it == nil { + panic("MapIter.SetValue called before Next") + } + iterelem := mapiterelem(it.it) + if iterelem == nil { + panic("MapIter.SetValue called on exhausted iterator") + } + + dst.mustBeAssignable() + var target unsafe.Pointer + if dst.kind() == Interface { + target = dst.ptr + } + + t := (*mapType)(unsafe.Pointer(it.m.typ)) + vtype := t.elem + + elem := Value{vtype, iterelem, it.m.flag | flag(vtype.Kind())} + elem = elem.assignTo("reflect.MapIter.SetValue", dst.typ, target) + typedmemmove(dst.typ, dst.ptr, elem.ptr) } // Next advances the map iterator and reports whether there is another -- cgit v1.2.3-54-g00ecf From de1c934b9709728b15cc821a055155ee13e1d0ab Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Fri, 20 Aug 2021 11:38:54 +0700 Subject: cmd/compile: fix checkptr false positive for (*[Big]T)(ptr)[:n:n] pattern The checkptr instrumentation is currently inserted before slice operation has validated that n <= Big. So instead of panic, checkptr have false positive throws. To fix this, just insert the checkptr instrumentation after the bound checking during SSA generation. Fixes #46938 Change-Id: I9dbf84441c711842ccc883f3654ca8766ac696d8 Reviewed-on: https://go-review.googlesource.com/c/go/+/343972 Trust: Cuong Manh Le Run-TryBot: Cuong Manh Le TryBot-Result: Go Bot Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/ir/expr.go | 9 +++++---- src/cmd/compile/internal/ssagen/ssa.go | 3 +++ src/cmd/compile/internal/walk/convert.go | 13 +++++++++++-- src/cmd/compile/internal/walk/expr.go | 2 +- test/fixedbugs/issue46938.go | 29 +++++++++++++++++++++++++++++ 5 files changed, 49 insertions(+), 7 deletions(-) create mode 100644 test/fixedbugs/issue46938.go diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go index f526d987a7..41de6bd61b 100644 --- a/src/cmd/compile/internal/ir/expr.go +++ b/src/cmd/compile/internal/ir/expr.go @@ -570,10 +570,11 @@ func (*SelectorExpr) CanBeNtype() {} // A SliceExpr is a slice expression X[Low:High] or X[Low:High:Max]. type SliceExpr struct { miniExpr - X Node - Low Node - High Node - Max Node + X Node + Low Node + High Node + Max Node + CheckPtrCall *CallExpr } func NewSliceExpr(pos src.XPos, op Op, x, low, high, max Node) *SliceExpr { diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go index 39d3b206ac..0a48f6b704 100644 --- a/src/cmd/compile/internal/ssagen/ssa.go +++ b/src/cmd/compile/internal/ssagen/ssa.go @@ -3090,6 +3090,9 @@ func (s *state) expr(n ir.Node) *ssa.Value { k = s.expr(n.Max) } p, l, c := s.slice(v, i, j, k, n.Bounded()) + if n.CheckPtrCall != nil { + s.stmt(n.CheckPtrCall) + } return s.newValue3(ssa.OpSliceMake, n.Type(), p, l, c) case ir.OSLICESTR: diff --git a/src/cmd/compile/internal/walk/convert.go b/src/cmd/compile/internal/walk/convert.go index 27a07ce4b6..f724ca7cae 100644 --- a/src/cmd/compile/internal/walk/convert.go +++ b/src/cmd/compile/internal/walk/convert.go @@ -413,11 +413,15 @@ func byteindex(n ir.Node) ir.Node { return n } -func walkCheckPtrAlignment(n *ir.ConvExpr, init *ir.Nodes, count ir.Node) ir.Node { +func walkCheckPtrAlignment(n *ir.ConvExpr, init *ir.Nodes, se *ir.SliceExpr) ir.Node { if !n.Type().IsPtr() { base.Fatalf("expected pointer type: %v", n.Type()) } elem := n.Type().Elem() + var count ir.Node + if se != nil { + count = se.Max + } if count != nil { if !elem.IsArray() { base.Fatalf("expected array type: %v", elem) @@ -435,7 +439,12 @@ func walkCheckPtrAlignment(n *ir.ConvExpr, init *ir.Nodes, count ir.Node) ir.Nod } n.X = cheapExpr(n.X, init) - init.Append(mkcall("checkptrAlignment", nil, init, typecheck.ConvNop(n.X, types.Types[types.TUNSAFEPTR]), reflectdata.TypePtr(elem), typecheck.Conv(count, types.Types[types.TUINTPTR]))) + checkPtrCall := mkcall("checkptrAlignment", nil, init, typecheck.ConvNop(n.X, types.Types[types.TUNSAFEPTR]), reflectdata.TypePtr(elem), typecheck.Conv(count, types.Types[types.TUINTPTR])) + if se != nil { + se.CheckPtrCall = checkPtrCall + } else { + init.Append(checkPtrCall) + } return n } diff --git a/src/cmd/compile/internal/walk/expr.go b/src/cmd/compile/internal/walk/expr.go index 26e225440a..c04998137b 100644 --- a/src/cmd/compile/internal/walk/expr.go +++ b/src/cmd/compile/internal/walk/expr.go @@ -824,7 +824,7 @@ func walkSlice(n *ir.SliceExpr, init *ir.Nodes) ir.Node { n.High = walkExpr(n.High, init) n.Max = walkExpr(n.Max, init) if checkSlice { - n.X = walkCheckPtrAlignment(n.X.(*ir.ConvExpr), init, n.Max) + n.X = walkCheckPtrAlignment(n.X.(*ir.ConvExpr), init, n) } if n.Op().IsSlice3() { diff --git a/test/fixedbugs/issue46938.go b/test/fixedbugs/issue46938.go new file mode 100644 index 0000000000..87532d4769 --- /dev/null +++ b/test/fixedbugs/issue46938.go @@ -0,0 +1,29 @@ +// run -gcflags="-d=checkptr" + +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "strings" + "unsafe" +) + +func main() { + defer func() { + err := recover() + if err == nil { + panic("expected panic") + } + if got := err.(error).Error(); !strings.Contains(got, "slice bounds out of range") { + panic("expected panic slice out of bound, got " + got) + } + }() + s := make([]int64, 100) + p := unsafe.Pointer(&s[0]) + n := 1000 + + _ = (*[10]int64)(p)[:n:n] +} -- cgit v1.2.3-54-g00ecf From 41b99dab0f263bd3fe5c2592f1c40735dcaa016a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 22 Jun 2021 20:26:31 -0700 Subject: os/user: don't skip TestLookupGroup if supported CL 37664 implemented this functionality, yet the tests were skipped. Introduce and use additional variable groupListImplemented to distinguish between these cases and enable TestLookupGroup for supported configurations (which looks like all but plan9). Change-Id: Iabaa7f08b4551dc67e67bdb6e715f15bb20d6218 Signed-off-by: Kir Kolyshkin Reviewed-on: https://go-review.googlesource.com/c/go/+/330751 Reviewed-by: Ian Lance Taylor Reviewed-by: Tobias Klauser --- src/os/user/lookup_plan9.go | 1 + src/os/user/lookup_stubs.go | 2 +- src/os/user/lookup_unix.go | 8 +++++--- src/os/user/user.go | 7 +++++-- src/os/user/user_test.go | 9 ++++++++- 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/os/user/lookup_plan9.go b/src/os/user/lookup_plan9.go index 51caf55935..07939363e7 100644 --- a/src/os/user/lookup_plan9.go +++ b/src/os/user/lookup_plan9.go @@ -20,6 +20,7 @@ const ( func init() { userImplemented = false groupImplemented = false + groupListImplemented = false } func current() (*User, error) { diff --git a/src/os/user/lookup_stubs.go b/src/os/user/lookup_stubs.go index c975a11964..d8e3d4866a 100644 --- a/src/os/user/lookup_stubs.go +++ b/src/os/user/lookup_stubs.go @@ -16,7 +16,7 @@ import ( ) func init() { - groupImplemented = false + groupListImplemented = false } func current() (*User, error) { diff --git a/src/os/user/lookup_unix.go b/src/os/user/lookup_unix.go index 97c611fad4..dffea4a885 100644 --- a/src/os/user/lookup_unix.go +++ b/src/os/user/lookup_unix.go @@ -18,13 +18,15 @@ import ( "strings" ) -const groupFile = "/etc/group" -const userFile = "/etc/passwd" +const ( + groupFile = "/etc/group" + userFile = "/etc/passwd" +) var colon = []byte{':'} func init() { - groupImplemented = false + groupListImplemented = false } // lineFunc returns a value, an error, or (nil, nil) to skip the row. diff --git a/src/os/user/user.go b/src/os/user/user.go index c1b8101c86..4e1b5b3407 100644 --- a/src/os/user/user.go +++ b/src/os/user/user.go @@ -20,9 +20,12 @@ import ( "strconv" ) +// These may be set to false in init() for a particular platform and/or +// build flags to let the tests know to skip tests of some features. var ( - userImplemented = true // set to false by lookup_stubs.go's init - groupImplemented = true // set to false by lookup_stubs.go's init + userImplemented = true + groupImplemented = true + groupListImplemented = true ) // User represents a user account. diff --git a/src/os/user/user_test.go b/src/os/user/user_test.go index 1112c78c00..d8a465edac 100644 --- a/src/os/user/user_test.go +++ b/src/os/user/user_test.go @@ -119,8 +119,15 @@ func TestLookupGroup(t *testing.T) { } } +func checkGroupList(t *testing.T) { + t.Helper() + if !groupListImplemented { + t.Skip("user: group list not implemented; skipping test") + } +} + func TestGroupIds(t *testing.T) { - checkGroup(t) + checkGroupList(t) if runtime.GOOS == "aix" { t.Skip("skipping GroupIds, see golang.org/issue/30563") } -- cgit v1.2.3-54-g00ecf From d37b8dedf7f96d88c7280f6afadb09b100380f93 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Sat, 21 Aug 2021 12:40:25 -0700 Subject: test: add test case that gofrontend miscompiled For #47771 Change-Id: I99dfdd48def756bde68445b50741afd6d86b6cf2 Reviewed-on: https://go-review.googlesource.com/c/go/+/344169 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Than McIntosh --- test/fixedbugs/issue47771.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 test/fixedbugs/issue47771.go diff --git a/test/fixedbugs/issue47771.go b/test/fixedbugs/issue47771.go new file mode 100644 index 0000000000..a434bffe4b --- /dev/null +++ b/test/fixedbugs/issue47771.go @@ -0,0 +1,19 @@ +// run + +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// gofrontend miscompiled some cases of append(s, make(typ, ln)...). + +package main + +var g int + +func main() { + a := []*int{&g, &g, &g, &g} + a = append(a[:0], make([]*int, len(a) - 1)...) + if len(a) != 3 || a[0] != nil || a[1] != nil || a[2] != nil { + panic(a) + } +} -- cgit v1.2.3-54-g00ecf From e1fcf8857e1b3e076cc3a6fad1860afe0d6c2ca6 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 20 Aug 2021 11:35:31 -0700 Subject: test: add test that caused gofrontend compiler crash Updates https://gcc.gnu.org/PR101994 Change-Id: I50dcb90e315792efd7d83b496034ad33b5f199e9 Reviewed-on: https://go-review.googlesource.com/c/go/+/343874 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Than McIntosh --- test/fixedbugs/gcc101994.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 test/fixedbugs/gcc101994.go diff --git a/test/fixedbugs/gcc101994.go b/test/fixedbugs/gcc101994.go new file mode 100644 index 0000000000..6e1e2b8075 --- /dev/null +++ b/test/fixedbugs/gcc101994.go @@ -0,0 +1,16 @@ +// compile + +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// https://gcc.gnu.org/PR101994 +// gccgo compiler crash with zero-sized result. + +package p + +type Empty struct{} + +func F() (int, Empty) { + return 0, Empty{} +} -- cgit v1.2.3-54-g00ecf From 099b819085e12ca45ac184cab5afb82538bec472 Mon Sep 17 00:00:00 2001 From: Dan Scales Date: Tue, 24 Aug 2021 15:34:52 -0700 Subject: cmd/compile: fix CheckSize() calculation for -G=3 and stencils Because the Align/Width of pointer types are always set when created, CalcSize() never descends past a pointer. Therefore, we need to do CheckSize() at every level when creating type. We need to do this for types creates by types2-to-types1 conversion and also by type substitution (mostly for stenciling). We also need to do Defer/ResumeCheckSize() at the top level in each of these cases to deal with potentially recursive types. These changes fix issue #47929 and also allow us to remove the special-case CheckSize() call that causes the problem for issue #47901. Fixes #47901 Fixes #47929 Change-Id: Icd8192431c145009cd6df2f4ade6db7da0f4dd3e Reviewed-on: https://go-review.googlesource.com/c/go/+/344829 Trust: Dan Scales Reviewed-by: Keith Randall --- src/cmd/compile/internal/noder/types.go | 24 ++++++-------- src/cmd/compile/internal/typecheck/subr.go | 52 ++++++++++++++++-------------- test/typeparam/issue47901.go | 21 ++++++++++++ test/typeparam/issue47929.go | 29 +++++++++++++++++ 4 files changed, 88 insertions(+), 38 deletions(-) create mode 100644 test/typeparam/issue47901.go create mode 100644 test/typeparam/issue47929.go diff --git a/src/cmd/compile/internal/noder/types.go b/src/cmd/compile/internal/noder/types.go index 541ed68ef3..c9f7c2bbe4 100644 --- a/src/cmd/compile/internal/noder/types.go +++ b/src/cmd/compile/internal/noder/types.go @@ -30,21 +30,11 @@ func (g *irgen) pkg(pkg *types2.Package) *types.Pkg { // typ converts a types2.Type to a types.Type, including caching of previously // translated types. func (g *irgen) typ(typ types2.Type) *types.Type { + // Defer the CheckSize calls until we have fully-defined a + // (possibly-recursive) top-level type. + types.DeferCheckSize() res := g.typ1(typ) - - // Calculate the size for all concrete types seen by the frontend. The old - // typechecker calls CheckSize() a lot, and we want to eliminate calling - // it eventually, so we should do it here instead. We only call it for - // top-level types (i.e. we do it here rather in typ1), to make sure that - // recursive types have been fully constructed before we call CheckSize. - if res != nil && !res.IsUntyped() && !res.IsFuncArgStruct() && !res.HasTParam() { - types.CheckSize(res) - if res.IsPtr() { - // Pointers always have their size set, even though their element - // may not have its size set. - types.CheckSize(res.Elem()) - } - } + types.ResumeCheckSize() return res } @@ -59,6 +49,12 @@ func (g *irgen) typ1(typ types2.Type) *types.Type { res, ok := g.typs[typ] if !ok { res = g.typ0(typ) + // Calculate the size for all concrete types seen by the frontend. + // This is the replacement for the CheckSize() calls in the types1 + // typechecker. These will be deferred until the top-level g.typ(). + if res != nil && !res.IsUntyped() && !res.IsFuncArgStruct() && !res.HasTParam() { + types.CheckSize(res) + } g.typs[typ] = res } return res diff --git a/src/cmd/compile/internal/typecheck/subr.go b/src/cmd/compile/internal/typecheck/subr.go index 7ae10ef406..b9cdcf10f2 100644 --- a/src/cmd/compile/internal/typecheck/subr.go +++ b/src/cmd/compile/internal/typecheck/subr.go @@ -1003,6 +1003,15 @@ type Tsubster struct { // result is t; otherwise the result is a new type. It deals with recursive types // by using TFORW types and finding partially or fully created types via sym.Def. func (ts *Tsubster) Typ(t *types.Type) *types.Type { + // Defer the CheckSize calls until we have fully-defined + // (possibly-recursive) top-level type. + types.DeferCheckSize() + r := ts.typ1(t) + types.ResumeCheckSize() + return r +} + +func (ts *Tsubster) typ1(t *types.Type) *types.Type { if !t.HasTParam() && t.Kind() != types.TFUNC { // Note: function types need to be copied regardless, as the // types of closures may contain declarations that need @@ -1047,7 +1056,7 @@ func (ts *Tsubster) Typ(t *types.Type) *types.Type { // the tparam/targs mapping from subst. neededTargs = make([]*types.Type, len(t.RParams())) for i, rparam := range t.RParams() { - neededTargs[i] = ts.Typ(rparam) + neededTargs[i] = ts.typ1(rparam) if !types.Identical(neededTargs[i], rparam) { targsChanged = true } @@ -1085,26 +1094,26 @@ func (ts *Tsubster) Typ(t *types.Type) *types.Type { } // Substitute the underlying typeparam (e.g. T in P[T], see // the example describing type P[T] above). - newt = ts.Typ(t.Underlying()) + newt = ts.typ1(t.Underlying()) assert(newt != t) case types.TARRAY: elem := t.Elem() - newelem := ts.Typ(elem) + newelem := ts.typ1(elem) if newelem != elem || targsChanged { newt = types.NewArray(newelem, t.NumElem()) } case types.TPTR: elem := t.Elem() - newelem := ts.Typ(elem) + newelem := ts.typ1(elem) if newelem != elem || targsChanged { newt = types.NewPtr(newelem) } case types.TSLICE: elem := t.Elem() - newelem := ts.Typ(elem) + newelem := ts.typ1(elem) if newelem != elem || targsChanged { newt = types.NewSlice(newelem) } @@ -1159,22 +1168,17 @@ func (ts *Tsubster) Typ(t *types.Type) *types.Type { } case types.TMAP: - newkey := ts.Typ(t.Key()) - newval := ts.Typ(t.Elem()) + newkey := ts.typ1(t.Key()) + newval := ts.typ1(t.Elem()) if newkey != t.Key() || newval != t.Elem() || targsChanged { newt = types.NewMap(newkey, newval) } case types.TCHAN: elem := t.Elem() - newelem := ts.Typ(elem) + newelem := ts.typ1(elem) if newelem != elem || targsChanged { newt = types.NewChan(newelem, t.ChanDir()) - if !newt.HasTParam() { - // TODO(danscales): not sure why I have to do this - // only for channels..... - types.CheckSize(newt) - } } case types.TFORW: if ts.SubstForwFunc != nil { @@ -1194,7 +1198,7 @@ func (ts *Tsubster) Typ(t *types.Type) *types.Type { for i := 0; i < nt; i++ { term, tilde := t.Term(i) tildes[i] = tilde - newterms[i] = ts.Typ(term) + newterms[i] = ts.typ1(term) if newterms[i] != term { changed = true } @@ -1212,24 +1216,24 @@ func (ts *Tsubster) Typ(t *types.Type) *types.Type { return t } - if t.Sym() == nil && t.Kind() != types.TINTER { - // Not a named type or interface type, so there was no forwarding type - // and there are no methods to substitute. - assert(t.Methods().Len() == 0) - return newt - } - if forw != nil { forw.SetUnderlying(newt) newt = forw } + if !newt.HasTParam() { + // Calculate the size of any new types created. These will be + // deferred until the top-level ts.Typ() or g.typ() (if this is + // called from g.fillinMethods()). + types.CheckSize(newt) + } + if t.Kind() != types.TINTER && t.Methods().Len() > 0 { // Fill in the method info for the new type. var newfields []*types.Field newfields = make([]*types.Field, t.Methods().Len()) for i, f := range t.Methods().Slice() { - t2 := ts.Typ(f.Type) + t2 := ts.typ1(f.Type) oldsym := f.Nname.Sym() newsym := MakeFuncInstSym(oldsym, ts.Targs, true) var nname *ir.Name @@ -1272,7 +1276,7 @@ func (ts *Tsubster) tstruct(t *types.Type, force bool) *types.Type { newfields = make([]*types.Field, t.NumFields()) } for i, f := range t.Fields().Slice() { - t2 := ts.Typ(f.Type) + t2 := ts.typ1(f.Type) if (t2 != f.Type || f.Nname != nil) && newfields == nil { newfields = make([]*types.Field, t.NumFields()) for j := 0; j < i; j++ { @@ -1325,7 +1329,7 @@ func (ts *Tsubster) tinter(t *types.Type) *types.Type { } var newfields []*types.Field for i, f := range t.Methods().Slice() { - t2 := ts.Typ(f.Type) + t2 := ts.typ1(f.Type) if (t2 != f.Type || f.Nname != nil) && newfields == nil { newfields = make([]*types.Field, t.Methods().Len()) for j := 0; j < i; j++ { diff --git a/test/typeparam/issue47901.go b/test/typeparam/issue47901.go new file mode 100644 index 0000000000..cd07973011 --- /dev/null +++ b/test/typeparam/issue47901.go @@ -0,0 +1,21 @@ +// run -gcflags=-G=3 + +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +type Chan[T any] chan Chan[T] + +func (ch Chan[T]) recv() Chan[T] { + return <-ch +} + +func main() { + ch := Chan[int](make(chan Chan[int])) + go func() { + ch <- make(Chan[int]) + }() + ch.recv() +} diff --git a/test/typeparam/issue47929.go b/test/typeparam/issue47929.go new file mode 100644 index 0000000000..a5636f2c7b --- /dev/null +++ b/test/typeparam/issue47929.go @@ -0,0 +1,29 @@ +// compile -G=3 -p=p + +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package v4 + +var sink interface{} + +//go:noinline +func Do(result, body interface{}) { + sink = &result +} + +func DataAction(result DataActionResponse, body DataActionRequest) { + Do(&result, body) +} + +type DataActionRequest struct { + Action *interface{} +} + +type DataActionResponse struct { + ValidationErrors *ValidationError +} + +type ValidationError struct { +} -- cgit v1.2.3-54-g00ecf From 08d4cc20cad0e95b4e368c2f38268199f9c68548 Mon Sep 17 00:00:00 2001 From: wdvxdr Date: Mon, 23 Aug 2021 00:48:10 +0800 Subject: cmd/compile: fix stencil call expression. Fixes: #47878 Change-Id: I369350813726fd518b4eab2b98f43bf031a6dee6 Reviewed-on: https://go-review.googlesource.com/c/go/+/344210 Reviewed-by: Dan Scales Trust: Dan Scales Trust: Keith Randall Run-TryBot: Dan Scales TryBot-Result: Go Bot --- src/cmd/compile/internal/noder/stencil.go | 7 +++++ test/typeparam/issue47878.go | 46 +++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 test/typeparam/issue47878.go diff --git a/src/cmd/compile/internal/noder/stencil.go b/src/cmd/compile/internal/noder/stencil.go index 18a0506036..b3ff4b8855 100644 --- a/src/cmd/compile/internal/noder/stencil.go +++ b/src/cmd/compile/internal/noder/stencil.go @@ -1048,6 +1048,13 @@ func (subst *subster) node(n ir.Node) ir.Node { case ir.OCLOSURE: transformCall(call) + case ir.ODEREF, ir.OINDEX, ir.OINDEXMAP, ir.ORECV: + // Transform a call that was delayed because of the + // use of typeparam inside an expression that required + // a pointer dereference, array indexing, map indexing, + // or channel receive to compute function value. + transformCall(call) + case ir.OFUNCINST: // A call with an OFUNCINST will get transformed // in stencil() once we have created & attached the diff --git a/test/typeparam/issue47878.go b/test/typeparam/issue47878.go new file mode 100644 index 0000000000..cb1043a440 --- /dev/null +++ b/test/typeparam/issue47878.go @@ -0,0 +1,46 @@ +// compile -G=3 + +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +type Src1[T any] func() Src1[T] + +func (s *Src1[T]) Next() { + *s = (*s)() +} + +type Src2[T any] []func() Src2[T] + +func (s Src2[T]) Next() { + _ = s[0]() +} + +type Src3[T comparable] map[T]func() Src3[T] + +func (s Src3[T]) Next() { + var a T + _ = s[a]() +} + +type Src4[T any] chan func() T + +func (s Src4[T]) Next() { + _ = (<-s)() +} + +func main() { + var src1 Src1[int] + src1.Next() + + var src2 Src2[int] + src2.Next() + + var src3 Src3[string] + src3.Next() + + var src4 Src4[int] + src4.Next() +} -- cgit v1.2.3-54-g00ecf From d2f002cb39bebdfac560282a43f3199c5d0903d7 Mon Sep 17 00:00:00 2001 From: korzhao Date: Sat, 21 Aug 2021 07:26:19 +0800 Subject: time/format: avoid growslice in time.String()/time.GoString() Pre-allocate the slice of buf with enough capacity to avoid growslice calls. benchmark old ns/op new ns/op delta BenchmarkTimeString-4 493 409 -17.12% BenchmarkTimeGoString-4 309 182 -41.30% benchmark old allocs new allocs delta BenchmarkTimeString-4 5 3 -40.00% BenchmarkTimeGoString-4 4 1 -75.00% benchmark old bytes new bytes delta BenchmarkTimeString-4 152 128 -15.79% BenchmarkTimeGoString-4 248 80 -67.74% Change-Id: I64eabe2ab0b3d4a846453c2e8e548a831d720b8c Reviewed-on: https://go-review.googlesource.com/c/go/+/343971 Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor Trust: Alexander Rakoczy --- src/time/format.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/time/format.go b/src/time/format.go index f4b4f48142..7ae89c557d 100644 --- a/src/time/format.go +++ b/src/time/format.go @@ -479,7 +479,7 @@ func (t Time) String() string { } m1, m2 := m2/1e9, m2%1e9 m0, m1 := m1/1e9, m1%1e9 - var buf []byte + buf := make([]byte, 0, 24) buf = append(buf, " m="...) buf = append(buf, sign) wid := 0 @@ -498,7 +498,8 @@ func (t Time) String() string { // GoString implements fmt.GoStringer and formats t to be printed in Go source // code. func (t Time) GoString() string { - buf := []byte("time.Date(") + buf := make([]byte, 0, 70) + buf = append(buf, "time.Date("...) buf = appendInt(buf, t.Year(), 0) month := t.Month() if January <= month && month <= December { -- cgit v1.2.3-54-g00ecf From 4f2ebfe34be7453ab144d82558cc4e735a55d644 Mon Sep 17 00:00:00 2001 From: korzhao Date: Tue, 17 Aug 2021 19:34:40 +0800 Subject: cmd/compile: allow embed into any byte slice type Fixes #47735 Change-Id: Ia21ea9a67f36a3edfef1b299ae4f3b00c306cd68 Reviewed-on: https://go-review.googlesource.com/c/go/+/342851 Reviewed-by: Matthew Dempsky Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Trust: Alexander Rakoczy --- src/cmd/compile/internal/staticdata/embed.go | 2 +- src/embed/internal/embedtest/embed_test.go | 40 ++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/cmd/compile/internal/staticdata/embed.go b/src/cmd/compile/internal/staticdata/embed.go index 0730d346b2..627c98ba44 100644 --- a/src/cmd/compile/internal/staticdata/embed.go +++ b/src/cmd/compile/internal/staticdata/embed.go @@ -73,7 +73,7 @@ func embedKind(typ *types.Type) int { if typ.Kind() == types.TSTRING { return embedString } - if typ.Sym() == nil && typ.IsSlice() && typ.Elem().Kind() == types.TUINT8 { + if typ.IsSlice() && typ.Elem().Kind() == types.TUINT8 { return embedBytes } return embedUnknown diff --git a/src/embed/internal/embedtest/embed_test.go b/src/embed/internal/embedtest/embed_test.go index 2d50f5e01f..b41359f4c2 100644 --- a/src/embed/internal/embedtest/embed_test.go +++ b/src/embed/internal/embedtest/embed_test.go @@ -129,3 +129,43 @@ func TestUninitialized(t *testing.T) { t.Errorf("in uninitialized embed.FS, . is not a directory") } } + +var ( + //go:embed "testdata/hello.txt" + helloT []T + //go:embed "testdata/hello.txt" + helloUint8 []uint8 + //go:embed "testdata/hello.txt" + helloEUint8 []EmbedUint8 + //go:embed "testdata/hello.txt" + helloBytes EmbedBytes + //go:embed "testdata/hello.txt" + helloString EmbedString +) + +type T byte +type EmbedUint8 uint8 +type EmbedBytes []byte +type EmbedString string + +// golang.org/issue/47735 +func TestAliases(t *testing.T) { + all := testDirAll + want, e := all.ReadFile("testdata/hello.txt") + if e != nil { + t.Fatal("ReadFile:", e) + } + check := func(g interface{}) { + got := reflect.ValueOf(g) + for i := 0; i < got.Len(); i++ { + if byte(got.Index(i).Uint()) != want[i] { + t.Fatalf("got %v want %v", got.Bytes(), want) + } + } + } + check(helloT) + check(helloUint8) + check(helloEUint8) + check(helloBytes) + check(helloString) +} -- cgit v1.2.3-54-g00ecf From 3d667671ad767d66bf792c5a8d623cb829f6366a Mon Sep 17 00:00:00 2001 From: korzhao Date: Wed, 25 Aug 2021 16:01:49 +0800 Subject: cmd/compile: fix function contains no TParam in generic function Fixes #47948 Change-Id: I446a9548265d195ae4d88aff6b1361474d1b6214 Reviewed-on: https://go-review.googlesource.com/c/go/+/344910 Trust: Alexander Rakoczy Trust: Dan Scales Run-TryBot: Alexander Rakoczy TryBot-Result: Go Bot Reviewed-by: Dan Scales --- src/cmd/compile/internal/typecheck/subr.go | 3 ++- test/typeparam/issue47948.go | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 test/typeparam/issue47948.go diff --git a/src/cmd/compile/internal/typecheck/subr.go b/src/cmd/compile/internal/typecheck/subr.go index b9cdcf10f2..8d05356543 100644 --- a/src/cmd/compile/internal/typecheck/subr.go +++ b/src/cmd/compile/internal/typecheck/subr.go @@ -1051,7 +1051,8 @@ func (ts *Tsubster) typ1(t *types.Type) *types.Type { var targsChanged bool var forw *types.Type - if t.Sym() != nil { + if t.Sym() != nil && t.HasTParam() { + // Need to test for t.HasTParam() again because of special TFUNC case above. // Translate the type params for this type according to // the tparam/targs mapping from subst. neededTargs = make([]*types.Type, len(t.RParams())) diff --git a/test/typeparam/issue47948.go b/test/typeparam/issue47948.go new file mode 100644 index 0000000000..8e5df81f6d --- /dev/null +++ b/test/typeparam/issue47948.go @@ -0,0 +1,18 @@ +// compile -G=3 + +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +type fun func() + +func F[T any]() { + _ = fun(func() { + + }) +} +func main() { + F[int]() +} -- cgit v1.2.3-54-g00ecf From 5baf60d47245c792c50a349cd6b8586d23204895 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Sat, 29 May 2021 19:11:37 -0700 Subject: bytes, strings: optimize Trim for single byte cutsets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using the latest version of all modules known by the module proxy, we determine that for all Trim usages (and related functionality): * 76.6% have cutsets of len=1, and * 13.4% have cutsets of len=2. Given that a vast majority of usages only have a cutset of len=1, we should more heavily optimize for that situation. Previously, there was some optimization for cutsets of len=1, but it's within the internal makeCutsetFunc function. This is sub-optimal as it incurs an allocation in makeCutsetFunc for the closure over that single byte. This CL removes special-casing of one-byte cutsets from makeCutsetFunc and instead distributes it directly in Trim, TrimRight, and TrimLeft. Whether we should distribute the entire ASCII cutset logic into Trim is a future CL that should be discussed and handled separately. The evidence for multibyte cutsets is not as obviously compelling. name old time/op new time/op delta bytes/TrimByte-4 84.1ns ± 2% 7.5ns ± 1% -91.10% (p=0.000 n=9+7) strings/TrimByte-4 86.2ns ± 3% 8.3ns ± 1% -90.33% (p=0.000 n=9+10) Fixes #46446 Change-Id: Ia0e31a8384c3ce111ae35465605bcec45df2ebec Reviewed-on: https://go-review.googlesource.com/c/go/+/323318 Trust: Joe Tsai Run-TryBot: Joe Tsai Reviewed-by: Brad Fitzpatrick TryBot-Result: Go Bot --- src/bytes/bytes.go | 28 +++++++++++++++++++++++----- src/bytes/bytes_test.go | 9 +++++++++ src/strings/strings.go | 28 +++++++++++++++++++++++----- src/strings/strings_test.go | 9 +++++++++ 4 files changed, 64 insertions(+), 10 deletions(-) diff --git a/src/bytes/bytes.go b/src/bytes/bytes.go index ce52649f13..cd859d086d 100644 --- a/src/bytes/bytes.go +++ b/src/bytes/bytes.go @@ -888,11 +888,6 @@ func (as *asciiSet) contains(c byte) bool { } func makeCutsetFunc(cutset string) func(r rune) bool { - if len(cutset) == 1 && cutset[0] < utf8.RuneSelf { - return func(r rune) bool { - return r == rune(cutset[0]) - } - } if as, isASCII := makeASCIISet(cutset); isASCII { return func(r rune) bool { return r < utf8.RuneSelf && as.contains(byte(r)) @@ -911,21 +906,44 @@ func makeCutsetFunc(cutset string) func(r rune) bool { // Trim returns a subslice of s by slicing off all leading and // trailing UTF-8-encoded code points contained in cutset. func Trim(s []byte, cutset string) []byte { + if len(cutset) == 1 && cutset[0] < utf8.RuneSelf { + return trimLeftByte(trimRightByte(s, cutset[0]), cutset[0]) + } return TrimFunc(s, makeCutsetFunc(cutset)) } // TrimLeft returns a subslice of s by slicing off all leading // UTF-8-encoded code points contained in cutset. func TrimLeft(s []byte, cutset string) []byte { + if len(cutset) == 1 && cutset[0] < utf8.RuneSelf { + return trimLeftByte(s, cutset[0]) + } return TrimLeftFunc(s, makeCutsetFunc(cutset)) } +func trimLeftByte(s []byte, c byte) []byte { + for len(s) > 0 && s[0] == c { + s = s[1:] + } + return s +} + // TrimRight returns a subslice of s by slicing off all trailing // UTF-8-encoded code points that are contained in cutset. func TrimRight(s []byte, cutset string) []byte { + if len(cutset) == 1 && cutset[0] < utf8.RuneSelf { + return trimRightByte(s, cutset[0]) + } return TrimRightFunc(s, makeCutsetFunc(cutset)) } +func trimRightByte(s []byte, c byte) []byte { + for len(s) > 0 && s[len(s)-1] == c { + s = s[:len(s)-1] + } + return s +} + // TrimSpace returns a subslice of s by slicing off all leading and // trailing white space, as defined by Unicode. func TrimSpace(s []byte) []byte { diff --git a/src/bytes/bytes_test.go b/src/bytes/bytes_test.go index 544ee46f90..850b2ed061 100644 --- a/src/bytes/bytes_test.go +++ b/src/bytes/bytes_test.go @@ -1251,7 +1251,9 @@ var trimTests = []TrimTest{ {"TrimLeft", "abba", "ab", ""}, {"TrimRight", "abba", "ab", ""}, {"TrimLeft", "abba", "a", "bba"}, + {"TrimLeft", "abba", "b", "abba"}, {"TrimRight", "abba", "a", "abb"}, + {"TrimRight", "abba", "b", "abba"}, {"Trim", "", "<>", "tag"}, {"Trim", "* listitem", " *", "listitem"}, {"Trim", `"quote"`, `"`, "quote"}, @@ -1963,6 +1965,13 @@ func BenchmarkTrimASCII(b *testing.B) { } } +func BenchmarkTrimByte(b *testing.B) { + x := []byte(" the quick brown fox ") + for i := 0; i < b.N; i++ { + Trim(x, " ") + } +} + func BenchmarkIndexPeriodic(b *testing.B) { key := []byte{1, 1} for _, skip := range [...]int{2, 4, 8, 16, 32, 64} { diff --git a/src/strings/strings.go b/src/strings/strings.go index b429735fea..0df8d2eb28 100644 --- a/src/strings/strings.go +++ b/src/strings/strings.go @@ -818,11 +818,6 @@ func (as *asciiSet) contains(c byte) bool { } func makeCutsetFunc(cutset string) func(rune) bool { - if len(cutset) == 1 && cutset[0] < utf8.RuneSelf { - return func(r rune) bool { - return r == rune(cutset[0]) - } - } if as, isASCII := makeASCIISet(cutset); isASCII { return func(r rune) bool { return r < utf8.RuneSelf && as.contains(byte(r)) @@ -837,6 +832,9 @@ func Trim(s, cutset string) string { if s == "" || cutset == "" { return s } + if len(cutset) == 1 && cutset[0] < utf8.RuneSelf { + return trimLeftByte(trimRightByte(s, cutset[0]), cutset[0]) + } return TrimFunc(s, makeCutsetFunc(cutset)) } @@ -848,9 +846,19 @@ func TrimLeft(s, cutset string) string { if s == "" || cutset == "" { return s } + if len(cutset) == 1 && cutset[0] < utf8.RuneSelf { + return trimLeftByte(s, cutset[0]) + } return TrimLeftFunc(s, makeCutsetFunc(cutset)) } +func trimLeftByte(s string, c byte) string { + for len(s) > 0 && s[0] == c { + s = s[1:] + } + return s +} + // TrimRight returns a slice of the string s, with all trailing // Unicode code points contained in cutset removed. // @@ -859,9 +867,19 @@ func TrimRight(s, cutset string) string { if s == "" || cutset == "" { return s } + if len(cutset) == 1 && cutset[0] < utf8.RuneSelf { + return trimRightByte(s, cutset[0]) + } return TrimRightFunc(s, makeCutsetFunc(cutset)) } +func trimRightByte(s string, c byte) string { + for len(s) > 0 && s[len(s)-1] == c { + s = s[:len(s)-1] + } + return s +} + // TrimSpace returns a slice of the string s, with all leading // and trailing white space removed, as defined by Unicode. func TrimSpace(s string) string { diff --git a/src/strings/strings_test.go b/src/strings/strings_test.go index 09e5b27cc3..edc6c20590 100644 --- a/src/strings/strings_test.go +++ b/src/strings/strings_test.go @@ -808,7 +808,9 @@ var trimTests = []struct { {"TrimLeft", "abba", "ab", ""}, {"TrimRight", "abba", "ab", ""}, {"TrimLeft", "abba", "a", "bba"}, + {"TrimLeft", "abba", "b", "abba"}, {"TrimRight", "abba", "a", "abb"}, + {"TrimRight", "abba", "b", "abba"}, {"Trim", "", "<>", "tag"}, {"Trim", "* listitem", " *", "listitem"}, {"Trim", `"quote"`, `"`, "quote"}, @@ -1860,6 +1862,13 @@ func BenchmarkTrimASCII(b *testing.B) { } } +func BenchmarkTrimByte(b *testing.B) { + x := " the quick brown fox " + for i := 0; i < b.N; i++ { + Trim(x, " ") + } +} + func BenchmarkIndexPeriodic(b *testing.B) { key := "aa" for _, skip := range [...]int{2, 4, 8, 16, 32, 64} { -- cgit v1.2.3-54-g00ecf From 6cf1d5d0fa1049e2910d048ce2b6b5a97fe6edc4 Mon Sep 17 00:00:00 2001 From: Jake Ciolek Date: Wed, 25 Aug 2021 12:36:17 +0200 Subject: cmd/compile: generic SSA rules for simplifying 2 and 3 operand integer arithmetic expressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This applies the following generic integer addition/subtraction transformations: x - (x + y) = -y (x - y) - x = -y y + (x - y) = x y + (z + (x - y) = x + z There's over 40 unique functions matching in Go. Hits 2 funcs in the runtime itself: runtime.stackfree() runtime.runqdrain() Go binary size reduced by 0.05% on Linux x86_64. StackCopy bench (perflocked Cascade Lake x86): name old time/op new time/op delta StackCopyPtr-8 87.3ms ± 1% 86.9ms ± 0% -0.45% (p=0.000 n=20+20) StackCopy-8 77.6ms ± 1% 77.0ms ± 0% -0.76% (p=0.000 n=20+20) StackCopyNoCache-8 2.28ms ± 2% 2.26ms ± 2% -0.93% (p=0.008 n=19+20) test/bench/go1 benchmarks (perflocked Cascade Lake x86): name old time/op new time/op delta BinaryTree17-8 1.88s ± 1% 1.88s ± 0% ~ (p=0.373 n=15+12) Fannkuch11-8 2.31s ± 0% 2.35s ± 0% +1.52% (p=0.000 n=15+14) FmtFprintfEmpty-8 26.6ns ± 0% 26.6ns ± 0% ~ (p=0.081 n=14+13) FmtFprintfString-8 48.6ns ± 0% 50.0ns ± 0% +2.86% (p=0.000 n=15+14) FmtFprintfInt-8 56.9ns ± 0% 54.8ns ± 0% -3.70% (p=0.000 n=15+15) FmtFprintfIntInt-8 90.4ns ± 0% 88.8ns ± 0% -1.78% (p=0.000 n=15+15) FmtFprintfPrefixedInt-8 104ns ± 0% 104ns ± 0% ~ (p=0.905 n=14+13) FmtFprintfFloat-8 148ns ± 0% 144ns ± 0% -2.19% (p=0.000 n=14+15) FmtManyArgs-8 389ns ± 0% 390ns ± 0% +0.35% (p=0.000 n=12+15) GobDecode-8 3.90ms ± 1% 3.88ms ± 0% -0.49% (p=0.000 n=15+14) GobEncode-8 2.73ms ± 0% 2.73ms ± 0% ~ (p=0.425 n=15+14) Gzip-8 169ms ± 0% 168ms ± 0% -0.52% (p=0.000 n=13+13) Gunzip-8 24.7ms ± 0% 24.8ms ± 0% +0.61% (p=0.000 n=15+15) HTTPClientServer-8 60.5µs ± 6% 60.4µs ± 7% ~ (p=0.595 n=15+15) JSONEncode-8 6.97ms ± 1% 6.93ms ± 0% -0.69% (p=0.000 n=14+14) JSONDecode-8 31.2ms ± 1% 30.8ms ± 1% -1.27% (p=0.000 n=14+14) Mandelbrot200-8 3.87ms ± 0% 3.87ms ± 0% ~ (p=0.652 n=15+14) GoParse-8 2.65ms ± 2% 2.64ms ± 1% ~ (p=0.202 n=15+15) RegexpMatchEasy0_32-8 45.1ns ± 0% 45.9ns ± 0% +1.68% (p=0.000 n=14+15) RegexpMatchEasy0_1K-8 140ns ± 0% 139ns ± 0% -0.44% (p=0.000 n=15+14) RegexpMatchEasy1_32-8 40.9ns ± 3% 40.5ns ± 0% -0.88% (p=0.000 n=15+13) RegexpMatchEasy1_1K-8 215ns ± 1% 220ns ± 1% +2.27% (p=0.000 n=15+15) RegexpMatchMedium_32-8 783ns ± 7% 738ns ± 0% ~ (p=0.361 n=15+15) RegexpMatchMedium_1K-8 24.1µs ± 6% 23.4µs ± 6% -2.94% (p=0.004 n=15+15) RegexpMatchHard_32-8 1.10µs ± 1% 1.09µs ± 1% -0.40% (p=0.006 n=15+14) RegexpMatchHard_1K-8 33.0µs ± 0% 33.0µs ± 0% ~ (p=0.535 n=12+14) Revcomp-8 354ms ± 0% 353ms ± 0% -0.23% (p=0.002 n=15+13) Template-8 42.0ms ± 1% 41.8ms ± 2% -0.37% (p=0.023 n=14+15) TimeParse-8 181ns ± 0% 180ns ± 1% -0.18% (p=0.014 n=12+13) TimeFormat-8 240ns ± 0% 242ns ± 1% +0.69% (p=0.000 n=12+15) [Geo mean] 35.2µs 35.1µs -0.43% name old speed new speed delta GobDecode-8 197MB/s ± 1% 198MB/s ± 0% +0.49% (p=0.000 n=15+14) GobEncode-8 281MB/s ± 0% 281MB/s ± 0% ~ (p=0.419 n=15+14) Gzip-8 115MB/s ± 0% 115MB/s ± 0% +0.52% (p=0.000 n=13+13) Gunzip-8 786MB/s ± 0% 781MB/s ± 0% -0.60% (p=0.000 n=15+15) JSONEncode-8 278MB/s ± 1% 280MB/s ± 0% +0.69% (p=0.000 n=14+14) JSONDecode-8 62.3MB/s ± 1% 63.1MB/s ± 1% +1.29% (p=0.000 n=14+14) GoParse-8 21.9MB/s ± 2% 22.0MB/s ± 1% ~ (p=0.205 n=15+15) RegexpMatchEasy0_32-8 709MB/s ± 0% 697MB/s ± 0% -1.65% (p=0.000 n=14+15) RegexpMatchEasy0_1K-8 7.34GB/s ± 0% 7.37GB/s ± 0% +0.43% (p=0.000 n=15+15) RegexpMatchEasy1_32-8 783MB/s ± 2% 790MB/s ± 0% +0.88% (p=0.000 n=15+13) RegexpMatchEasy1_1K-8 4.77GB/s ± 1% 4.66GB/s ± 1% -2.23% (p=0.000 n=15+15) RegexpMatchMedium_32-8 41.0MB/s ± 7% 43.3MB/s ± 0% ~ (p=0.360 n=15+15) RegexpMatchMedium_1K-8 42.5MB/s ± 6% 43.8MB/s ± 6% +3.07% (p=0.004 n=15+15) RegexpMatchHard_32-8 29.2MB/s ± 1% 29.3MB/s ± 1% +0.41% (p=0.006 n=15+14) RegexpMatchHard_1K-8 31.1MB/s ± 0% 31.1MB/s ± 0% ~ (p=0.495 n=12+14) Revcomp-8 718MB/s ± 0% 720MB/s ± 0% +0.23% (p=0.002 n=15+13) Template-8 46.3MB/s ± 1% 46.4MB/s ± 2% +0.38% (p=0.021 n=14+15) [Geo mean] 205MB/s 206MB/s +0.57% Change-Id: Ibd1afdf8b6c0b08087dcc3acd8f943637eb95ac0 Reviewed-on: https://go-review.googlesource.com/c/go/+/344930 Reviewed-by: Keith Randall Run-TryBot: Keith Randall TryBot-Result: Go Bot Trust: Josh Bleecher Snyder --- src/cmd/compile/internal/ssa/gen/generic.rules | 4 + src/cmd/compile/internal/ssa/rewritegeneric.go | 328 +++++++++++++++++++++++++ test/codegen/arithmetic.go | 24 ++ 3 files changed, 356 insertions(+) diff --git a/src/cmd/compile/internal/ssa/gen/generic.rules b/src/cmd/compile/internal/ssa/gen/generic.rules index 5cbc70cf41..e7ee3d0efd 100644 --- a/src/cmd/compile/internal/ssa/gen/generic.rules +++ b/src/cmd/compile/internal/ssa/gen/generic.rules @@ -590,6 +590,10 @@ // simplifications often used for lengths. e.g. len(s[i:i+5])==5 (Sub(64|32|16|8) (Add(64|32|16|8) x y) x) => y (Sub(64|32|16|8) (Add(64|32|16|8) x y) y) => x +(Sub(64|32|16|8) (Sub(64|32|16|8) x y) x) => (Neg(64|32|16|8) y) +(Sub(64|32|16|8) x (Add(64|32|16|8) x y)) => (Neg(64|32|16|8) y) +(Add(64|32|16|8) x (Sub(64|32|16|8) y x)) => y +(Add(64|32|16|8) x (Add(64|32|16|8) y (Sub(64|32|16|8) z x))) => (Add(64|32|16|8) y z) // basic phi simplifications (Phi (Const8 [c]) (Const8 [c])) => (Const8 [c]) diff --git a/src/cmd/compile/internal/ssa/rewritegeneric.go b/src/cmd/compile/internal/ssa/rewritegeneric.go index 52258201ca..e2f9e3ebba 100644 --- a/src/cmd/compile/internal/ssa/rewritegeneric.go +++ b/src/cmd/compile/internal/ssa/rewritegeneric.go @@ -533,6 +533,52 @@ func rewriteValuegeneric_OpAdd16(v *Value) bool { } break } + // match: (Add16 x (Sub16 y x)) + // result: y + for { + for _i0 := 0; _i0 <= 1; _i0, v_0, v_1 = _i0+1, v_1, v_0 { + x := v_0 + if v_1.Op != OpSub16 { + continue + } + _ = v_1.Args[1] + y := v_1.Args[0] + if x != v_1.Args[1] { + continue + } + v.copyOf(y) + return true + } + break + } + // match: (Add16 x (Add16 y (Sub16 z x))) + // result: (Add16 y z) + for { + for _i0 := 0; _i0 <= 1; _i0, v_0, v_1 = _i0+1, v_1, v_0 { + x := v_0 + if v_1.Op != OpAdd16 { + continue + } + _ = v_1.Args[1] + v_1_0 := v_1.Args[0] + v_1_1 := v_1.Args[1] + for _i1 := 0; _i1 <= 1; _i1, v_1_0, v_1_1 = _i1+1, v_1_1, v_1_0 { + y := v_1_0 + if v_1_1.Op != OpSub16 { + continue + } + _ = v_1_1.Args[1] + z := v_1_1.Args[0] + if x != v_1_1.Args[1] { + continue + } + v.reset(OpAdd16) + v.AddArg2(y, z) + return true + } + } + break + } // match: (Add16 (Add16 i:(Const16 ) z) x) // cond: (z.Op != OpConst16 && x.Op != OpConst16) // result: (Add16 i (Add16 z x)) @@ -732,6 +778,52 @@ func rewriteValuegeneric_OpAdd32(v *Value) bool { } break } + // match: (Add32 x (Sub32 y x)) + // result: y + for { + for _i0 := 0; _i0 <= 1; _i0, v_0, v_1 = _i0+1, v_1, v_0 { + x := v_0 + if v_1.Op != OpSub32 { + continue + } + _ = v_1.Args[1] + y := v_1.Args[0] + if x != v_1.Args[1] { + continue + } + v.copyOf(y) + return true + } + break + } + // match: (Add32 x (Add32 y (Sub32 z x))) + // result: (Add32 y z) + for { + for _i0 := 0; _i0 <= 1; _i0, v_0, v_1 = _i0+1, v_1, v_0 { + x := v_0 + if v_1.Op != OpAdd32 { + continue + } + _ = v_1.Args[1] + v_1_0 := v_1.Args[0] + v_1_1 := v_1.Args[1] + for _i1 := 0; _i1 <= 1; _i1, v_1_0, v_1_1 = _i1+1, v_1_1, v_1_0 { + y := v_1_0 + if v_1_1.Op != OpSub32 { + continue + } + _ = v_1_1.Args[1] + z := v_1_1.Args[0] + if x != v_1_1.Args[1] { + continue + } + v.reset(OpAdd32) + v.AddArg2(y, z) + return true + } + } + break + } // match: (Add32 (Add32 i:(Const32 ) z) x) // cond: (z.Op != OpConst32 && x.Op != OpConst32) // result: (Add32 i (Add32 z x)) @@ -958,6 +1050,52 @@ func rewriteValuegeneric_OpAdd64(v *Value) bool { } break } + // match: (Add64 x (Sub64 y x)) + // result: y + for { + for _i0 := 0; _i0 <= 1; _i0, v_0, v_1 = _i0+1, v_1, v_0 { + x := v_0 + if v_1.Op != OpSub64 { + continue + } + _ = v_1.Args[1] + y := v_1.Args[0] + if x != v_1.Args[1] { + continue + } + v.copyOf(y) + return true + } + break + } + // match: (Add64 x (Add64 y (Sub64 z x))) + // result: (Add64 y z) + for { + for _i0 := 0; _i0 <= 1; _i0, v_0, v_1 = _i0+1, v_1, v_0 { + x := v_0 + if v_1.Op != OpAdd64 { + continue + } + _ = v_1.Args[1] + v_1_0 := v_1.Args[0] + v_1_1 := v_1.Args[1] + for _i1 := 0; _i1 <= 1; _i1, v_1_0, v_1_1 = _i1+1, v_1_1, v_1_0 { + y := v_1_0 + if v_1_1.Op != OpSub64 { + continue + } + _ = v_1_1.Args[1] + z := v_1_1.Args[0] + if x != v_1_1.Args[1] { + continue + } + v.reset(OpAdd64) + v.AddArg2(y, z) + return true + } + } + break + } // match: (Add64 (Add64 i:(Const64 ) z) x) // cond: (z.Op != OpConst64 && x.Op != OpConst64) // result: (Add64 i (Add64 z x)) @@ -1184,6 +1322,52 @@ func rewriteValuegeneric_OpAdd8(v *Value) bool { } break } + // match: (Add8 x (Sub8 y x)) + // result: y + for { + for _i0 := 0; _i0 <= 1; _i0, v_0, v_1 = _i0+1, v_1, v_0 { + x := v_0 + if v_1.Op != OpSub8 { + continue + } + _ = v_1.Args[1] + y := v_1.Args[0] + if x != v_1.Args[1] { + continue + } + v.copyOf(y) + return true + } + break + } + // match: (Add8 x (Add8 y (Sub8 z x))) + // result: (Add8 y z) + for { + for _i0 := 0; _i0 <= 1; _i0, v_0, v_1 = _i0+1, v_1, v_0 { + x := v_0 + if v_1.Op != OpAdd8 { + continue + } + _ = v_1.Args[1] + v_1_0 := v_1.Args[0] + v_1_1 := v_1.Args[1] + for _i1 := 0; _i1 <= 1; _i1, v_1_0, v_1_1 = _i1+1, v_1_1, v_1_0 { + y := v_1_0 + if v_1_1.Op != OpSub8 { + continue + } + _ = v_1_1.Args[1] + z := v_1_1.Args[0] + if x != v_1_1.Args[1] { + continue + } + v.reset(OpAdd8) + v.AddArg2(y, z) + return true + } + } + break + } // match: (Add8 (Add8 i:(Const8 ) z) x) // cond: (z.Op != OpConst8 && x.Op != OpConst8) // result: (Add8 i (Add8 z x)) @@ -22590,6 +22774,42 @@ func rewriteValuegeneric_OpSub16(v *Value) bool { } break } + // match: (Sub16 (Sub16 x y) x) + // result: (Neg16 y) + for { + if v_0.Op != OpSub16 { + break + } + y := v_0.Args[1] + x := v_0.Args[0] + if x != v_1 { + break + } + v.reset(OpNeg16) + v.AddArg(y) + return true + } + // match: (Sub16 x (Add16 x y)) + // result: (Neg16 y) + for { + x := v_0 + if v_1.Op != OpAdd16 { + break + } + _ = v_1.Args[1] + v_1_0 := v_1.Args[0] + v_1_1 := v_1.Args[1] + for _i0 := 0; _i0 <= 1; _i0, v_1_0, v_1_1 = _i0+1, v_1_1, v_1_0 { + if x != v_1_0 { + continue + } + y := v_1_1 + v.reset(OpNeg16) + v.AddArg(y) + return true + } + break + } // match: (Sub16 x (Sub16 i:(Const16 ) z)) // cond: (z.Op != OpConst16 && x.Op != OpConst16) // result: (Sub16 (Add16 x z) i) @@ -22869,6 +23089,42 @@ func rewriteValuegeneric_OpSub32(v *Value) bool { } break } + // match: (Sub32 (Sub32 x y) x) + // result: (Neg32 y) + for { + if v_0.Op != OpSub32 { + break + } + y := v_0.Args[1] + x := v_0.Args[0] + if x != v_1 { + break + } + v.reset(OpNeg32) + v.AddArg(y) + return true + } + // match: (Sub32 x (Add32 x y)) + // result: (Neg32 y) + for { + x := v_0 + if v_1.Op != OpAdd32 { + break + } + _ = v_1.Args[1] + v_1_0 := v_1.Args[0] + v_1_1 := v_1.Args[1] + for _i0 := 0; _i0 <= 1; _i0, v_1_0, v_1_1 = _i0+1, v_1_1, v_1_0 { + if x != v_1_0 { + continue + } + y := v_1_1 + v.reset(OpNeg32) + v.AddArg(y) + return true + } + break + } // match: (Sub32 x (Sub32 i:(Const32 ) z)) // cond: (z.Op != OpConst32 && x.Op != OpConst32) // result: (Sub32 (Add32 x z) i) @@ -23172,6 +23428,42 @@ func rewriteValuegeneric_OpSub64(v *Value) bool { } break } + // match: (Sub64 (Sub64 x y) x) + // result: (Neg64 y) + for { + if v_0.Op != OpSub64 { + break + } + y := v_0.Args[1] + x := v_0.Args[0] + if x != v_1 { + break + } + v.reset(OpNeg64) + v.AddArg(y) + return true + } + // match: (Sub64 x (Add64 x y)) + // result: (Neg64 y) + for { + x := v_0 + if v_1.Op != OpAdd64 { + break + } + _ = v_1.Args[1] + v_1_0 := v_1.Args[0] + v_1_1 := v_1.Args[1] + for _i0 := 0; _i0 <= 1; _i0, v_1_0, v_1_1 = _i0+1, v_1_1, v_1_0 { + if x != v_1_0 { + continue + } + y := v_1_1 + v.reset(OpNeg64) + v.AddArg(y) + return true + } + break + } // match: (Sub64 x (Sub64 i:(Const64 ) z)) // cond: (z.Op != OpConst64 && x.Op != OpConst64) // result: (Sub64 (Add64 x z) i) @@ -23475,6 +23767,42 @@ func rewriteValuegeneric_OpSub8(v *Value) bool { } break } + // match: (Sub8 (Sub8 x y) x) + // result: (Neg8 y) + for { + if v_0.Op != OpSub8 { + break + } + y := v_0.Args[1] + x := v_0.Args[0] + if x != v_1 { + break + } + v.reset(OpNeg8) + v.AddArg(y) + return true + } + // match: (Sub8 x (Add8 x y)) + // result: (Neg8 y) + for { + x := v_0 + if v_1.Op != OpAdd8 { + break + } + _ = v_1.Args[1] + v_1_0 := v_1.Args[0] + v_1_1 := v_1.Args[1] + for _i0 := 0; _i0 <= 1; _i0, v_1_0, v_1_1 = _i0+1, v_1_1, v_1_0 { + if x != v_1_0 { + continue + } + y := v_1_1 + v.reset(OpNeg8) + v.AddArg(y) + return true + } + break + } // match: (Sub8 x (Sub8 i:(Const8 ) z)) // cond: (z.Op != OpConst8 && x.Op != OpConst8) // result: (Sub8 (Add8 x z) i) diff --git a/test/codegen/arithmetic.go b/test/codegen/arithmetic.go index eb0f338036..eb95416b6a 100644 --- a/test/codegen/arithmetic.go +++ b/test/codegen/arithmetic.go @@ -84,6 +84,30 @@ func NegAddFromConstNeg(a int) int { return c } +func SubSubNegSimplify(a, b int) int { + // amd64:"NEGQ" + r := (a - b) - a + return r +} + +func SubAddSimplify(a, b int) int { + // amd64:-"SUBQ",-"ADDQ" + r := a + (b - a) + return r +} + +func SubAddNegSimplify(a, b int) int { + // amd64:"NEGQ",-"ADDQ",-"SUBQ" + r := a - (b + a) + return r +} + +func AddAddSubSimplify(a, b, c int) int { + // amd64:-"SUBQ" + r := a + (b + (c - a)) + return r +} + // -------------------- // // Multiplication // // -------------------- // -- cgit v1.2.3-54-g00ecf From 647bef6c59e201792688d88cdc50ea0c6a68990b Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 24 Aug 2021 09:28:43 -0700 Subject: go/types: implement NewTypeList and use it instead of composite literals Also, simplify a bit of code in predicates.go. This is a backport of changes in CL 344615 that were made in addition to the changes of the original CL 343933; it brings go/types in sync with types2. Change-Id: I14cd4d4704d29894d0fbb8d129744d65e332ad22 Reviewed-on: https://go-review.googlesource.com/c/go/+/344570 Trust: Robert Griesemer Trust: Dan Scales Run-TryBot: Robert Griesemer TryBot-Result: Go Bot Reviewed-by: Dan Scales --- src/go/types/check.go | 2 +- src/go/types/instantiate.go | 2 +- src/go/types/predicates.go | 8 +++----- src/go/types/subst.go | 2 +- src/go/types/typelists.go | 8 ++++++++ 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/go/types/check.go b/src/go/types/check.go index 909bf8d52d..ab3a388e9f 100644 --- a/src/go/types/check.go +++ b/src/go/types/check.go @@ -407,7 +407,7 @@ func (check *Checker) recordInferred(call ast.Expr, targs []Type, sig *Signature assert(call != nil) assert(sig != nil) if m := check.Inferred; m != nil { - m[call] = Inferred{&TypeList{targs}, sig} + m[call] = Inferred{NewTypeList(targs), sig} } } diff --git a/src/go/types/instantiate.go b/src/go/types/instantiate.go index 3ee09b7e84..5f691d5246 100644 --- a/src/go/types/instantiate.go +++ b/src/go/types/instantiate.go @@ -131,7 +131,7 @@ func (check *Checker) instance(pos token.Pos, typ Type, targs []Type) (res Type) tname := NewTypeName(pos, t.obj.pkg, t.obj.name, nil) named := check.newNamed(tname, t, nil, nil, nil) // methods and tparams are set when named is loaded - named.targs = &TypeList{targs} + named.targs = NewTypeList(targs) named.instance = &instance{pos} if check != nil { check.typMap[h] = named diff --git a/src/go/types/predicates.go b/src/go/types/predicates.go index 2f4ef9dace..d4055bb0cc 100644 --- a/src/go/types/predicates.go +++ b/src/go/types/predicates.go @@ -312,16 +312,14 @@ func identical(x, y Type, cmpTags bool, p *ifacePair) bool { return false } - if nargs := len(xargs); nargs > 0 { + if len(xargs) > 0 { // Instances are identical if their original type and type arguments // are identical. if !Identical(x.orig, y.orig) { return false } - for i := 0; i < nargs; i++ { - xa := xargs[i] - ya := yargs[i] - if !Identical(xa, ya) { + for i, xa := range xargs { + if !Identical(xa, yargs[i]) { return false } } diff --git a/src/go/types/subst.go b/src/go/types/subst.go index 8b8d6fb82a..1c53cdaf2c 100644 --- a/src/go/types/subst.go +++ b/src/go/types/subst.go @@ -233,7 +233,7 @@ func (subst *subster) typ(typ Type) Type { // It's ok to provide a nil *Checker because the newly created type // doesn't need to be (lazily) expanded; it's expanded below. named := (*Checker)(nil).newNamed(tname, t.orig, nil, t.tparams, t.methods) // t is loaded, so tparams and methods are available - named.targs = &TypeList{newTArgs} + named.targs = NewTypeList(newTArgs) subst.typMap[h] = named t.expand(subst.typMap) // must happen after typMap update to avoid infinite recursion diff --git a/src/go/types/typelists.go b/src/go/types/typelists.go index a8181404bf..ef8ea1f32b 100644 --- a/src/go/types/typelists.go +++ b/src/go/types/typelists.go @@ -27,6 +27,14 @@ func (l *TParamList) list() []*TypeParam { // TypeList holds a list of types. type TypeList struct{ types []Type } +// NewTypeList returns a new TypeList with the types in list. +func NewTypeList(list []Type) *TypeList { + if len(list) == 0 { + return nil + } + return &TypeList{list} +} + // Len returns the number of types in the list. // It is safe to call on a nil receiver. func (l *TypeList) Len() int { return len(l.list()) } -- cgit v1.2.3-54-g00ecf From 4158e88f64f34d1d0bab1d54be6be72a598ca41f Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 24 Aug 2021 12:23:28 -0700 Subject: cmd/compile/internal/syntax: fix position of type parameter field Change-Id: I8bca01b935301e7bd4efa55ed21921dbf31a75b9 Reviewed-on: https://go-review.googlesource.com/c/go/+/344575 Trust: Robert Griesemer Run-TryBot: Robert Griesemer TryBot-Result: Go Bot Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/syntax/parser.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cmd/compile/internal/syntax/parser.go b/src/cmd/compile/internal/syntax/parser.go index c477ddd45d..fd97279f9d 100644 --- a/src/cmd/compile/internal/syntax/parser.go +++ b/src/cmd/compile/internal/syntax/parser.go @@ -1840,7 +1840,11 @@ func (p *parser) paramDeclOrNil(name *Name) *Field { } f := new(Field) - f.pos = p.pos() + if name != nil { + f.pos = name.pos + } else { + f.pos = p.pos() + } if p.tok == _Name || name != nil { if name == nil { -- cgit v1.2.3-54-g00ecf From bf0bc4122fd4b3a75c2f9c107895cd5e2f89b90e Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 24 Aug 2021 13:52:07 -0700 Subject: go/types, types2: don't re-evaluate context string for each function argument (optimization) Change-Id: Ie1b4d5b64350ea42484adea14df84cacd1d2653b Reviewed-on: https://go-review.googlesource.com/c/go/+/344576 Trust: Robert Griesemer Trust: Dan Scales Run-TryBot: Robert Griesemer TryBot-Result: Go Bot Reviewed-by: Dan Scales --- src/cmd/compile/internal/types2/call.go | 7 +++++-- src/go/types/call.go | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/cmd/compile/internal/types2/call.go b/src/cmd/compile/internal/types2/call.go index 538fdc0fb7..4bbc524856 100644 --- a/src/cmd/compile/internal/types2/call.go +++ b/src/cmd/compile/internal/types2/call.go @@ -341,8 +341,11 @@ func (check *Checker) arguments(call *syntax.CallExpr, sig *Signature, targs []T } // check arguments - for i, a := range args { - check.assignment(a, sigParams.vars[i].typ, check.sprintf("argument to %s", call.Fun)) + if len(args) > 0 { + context := check.sprintf("argument to %s", call.Fun) + for i, a := range args { + check.assignment(a, sigParams.vars[i].typ, context) + } } return diff --git a/src/go/types/call.go b/src/go/types/call.go index 87eeef444b..fdecafb781 100644 --- a/src/go/types/call.go +++ b/src/go/types/call.go @@ -347,8 +347,11 @@ func (check *Checker) arguments(call *ast.CallExpr, sig *Signature, targs []Type } // check arguments - for i, a := range args { - check.assignment(a, sigParams.vars[i].typ, check.sprintf("argument to %s", call.Fun)) + if len(args) > 0 { + context := check.sprintf("argument to %s", call.Fun) + for i, a := range args { + check.assignment(a, sigParams.vars[i].typ, context) + } } return -- cgit v1.2.3-54-g00ecf From 4068fb6c2162b38db7912903ff12bafe9f5ca9bb Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 24 Aug 2021 18:07:42 -0700 Subject: cmd/compile: always accept 1.18 syntax but complain if not 1.18 This CL configures the parser to always accept 1.18 syntax (type parameters, type instantiations, interface elements), even when -lang is set to an earlier release. Instead, the type checker looks for 1.18 operations and complains if the language version is set to an earlier release. Doing these checks during type checking is necessary because it it is possible to write "generic" code using pre-1.18 syntax; for instance, an imported generic function may be implicitly instantiated (as in imported.Max(2, 3)), or an imported constraint interface may be embedded in an "ordinary" interface. Fixes #47818. Change-Id: I83ec302b3f4ba7196c0a4743c03670cfb901310d Reviewed-on: https://go-review.googlesource.com/c/go/+/344871 Trust: Robert Griesemer Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/noder/noder.go | 2 +- src/cmd/compile/internal/types2/call.go | 11 ++++ src/cmd/compile/internal/types2/decl.go | 23 +++++++-- src/cmd/compile/internal/types2/resolver.go | 40 +++++++++------ src/cmd/compile/internal/types2/subst.go | 2 +- .../internal/types2/testdata/check/decls0.src | 2 +- .../internal/types2/testdata/check/issues.src | 6 +-- .../internal/types2/testdata/check/main.go2 | 2 +- .../internal/types2/testdata/check/typeparams.go2 | 4 +- .../types2/testdata/fixedbugs/issue47818.go2 | 59 ++++++++++++++++++++++ src/cmd/compile/internal/types2/typeset.go | 11 +++- src/cmd/compile/internal/types2/typexpr.go | 15 +++--- test/fixedbugs/issue10975.go | 2 +- 13 files changed, 142 insertions(+), 37 deletions(-) create mode 100644 src/cmd/compile/internal/types2/testdata/fixedbugs/issue47818.go2 diff --git a/src/cmd/compile/internal/noder/noder.go b/src/cmd/compile/internal/noder/noder.go index 2b67a91b3f..e1b485b2b3 100644 --- a/src/cmd/compile/internal/noder/noder.go +++ b/src/cmd/compile/internal/noder/noder.go @@ -35,7 +35,7 @@ func LoadPackage(filenames []string) { supportsGenerics := base.Flag.G != 0 || buildcfg.Experiment.Unified mode := syntax.CheckBranches - if supportsGenerics && types.AllowsGoVersion(types.LocalPkg, 1, 18) { + if supportsGenerics { mode |= syntax.AllowGenerics } diff --git a/src/cmd/compile/internal/types2/call.go b/src/cmd/compile/internal/types2/call.go index 4bbc524856..0b062b4c94 100644 --- a/src/cmd/compile/internal/types2/call.go +++ b/src/cmd/compile/internal/types2/call.go @@ -15,6 +15,10 @@ import ( // funcInst type-checks a function instantiation inst and returns the result in x. // The operand x must be the evaluation of inst.X and its type must be a signature. func (check *Checker) funcInst(x *operand, inst *syntax.IndexExpr) { + if !check.allowVersion(check.pkg, 1, 18) { + check.softErrorf(inst.Pos(), "function instantiation requires go1.18 or later") + } + xlist := unpackExpr(inst.Index) targs := check.typeList(xlist) if targs == nil { @@ -318,6 +322,13 @@ func (check *Checker) arguments(call *syntax.CallExpr, sig *Signature, targs []T // infer type arguments and instantiate signature if necessary if sig.TParams().Len() > 0 { + if !check.allowVersion(check.pkg, 1, 18) { + if iexpr, _ := call.Fun.(*syntax.IndexExpr); iexpr != nil { + check.softErrorf(iexpr.Pos(), "function instantiation requires go1.18 or later") + } else { + check.softErrorf(call.Pos(), "implicit function instantiation requires go1.18 or later") + } + } // TODO(gri) provide position information for targs so we can feed // it to the instantiate call for better error reporting targs := check.infer(call.Pos(), sig.TParams().list(), targs, sigParams, args, true) diff --git a/src/cmd/compile/internal/types2/decl.go b/src/cmd/compile/internal/types2/decl.go index 342e1090de..d7a33546aa 100644 --- a/src/cmd/compile/internal/types2/decl.go +++ b/src/cmd/compile/internal/types2/decl.go @@ -514,11 +514,26 @@ func (check *Checker) varDecl(obj *Var, lhs []*Var, typ, init syntax.Expr) { check.initVars(lhs, []syntax.Expr{init}, nopos) } +// isImportedConstraint reports whether typ is an imported type constraint. +func (check *Checker) isImportedConstraint(typ Type) bool { + named, _ := typ.(*Named) + if named == nil || named.obj.pkg == check.pkg || named.obj.pkg == nil { + return false + } + u, _ := named.under().(*Interface) + return u != nil && u.IsConstraint() +} + func (check *Checker) typeDecl(obj *TypeName, tdecl *syntax.TypeDecl, def *Named) { assert(obj.typ == nil) + var rhs Type check.later(func() { check.validType(obj.typ, nil) + // If typ is local, an error was already reported where typ is specified/defined. + if check.isImportedConstraint(rhs) && !check.allowVersion(check.pkg, 1, 18) { + check.errorf(tdecl.Type.Pos(), "using type constraint %s requires go1.18 or later", rhs) + } }) alias := tdecl.Alias @@ -540,7 +555,8 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *syntax.TypeDecl, def *Named } obj.typ = Typ[Invalid] - obj.typ = check.anyType(tdecl.Type) + rhs = check.anyType(tdecl.Type) + obj.typ = rhs return } @@ -555,8 +571,9 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *syntax.TypeDecl, def *Named } // determine underlying type of named - named.fromRHS = check.definedType(tdecl.Type, named) - assert(named.fromRHS != nil) + rhs = check.definedType(tdecl.Type, named) + assert(rhs != nil) + named.fromRHS = rhs // The underlying type of named may be itself a named type that is // incomplete: // diff --git a/src/cmd/compile/internal/types2/resolver.go b/src/cmd/compile/internal/types2/resolver.go index 018a20cfb2..34fbc3d41b 100644 --- a/src/cmd/compile/internal/types2/resolver.go +++ b/src/cmd/compile/internal/types2/resolver.go @@ -412,52 +412,60 @@ func (check *Checker) collectObjects() { } case *syntax.TypeDecl: + if len(s.TParamList) != 0 && !check.allowVersion(pkg, 1, 18) { + check.softErrorf(s.TParamList[0], "type parameters require go1.18 or later") + } obj := NewTypeName(s.Name.Pos(), pkg, s.Name.Value, nil) check.declarePkgObj(s.Name, obj, &declInfo{file: fileScope, tdecl: s}) case *syntax.FuncDecl: - d := s // TODO(gri) get rid of this - name := d.Name.Value - obj := NewFunc(d.Name.Pos(), pkg, name, nil) - if d.Recv == nil { + name := s.Name.Value + obj := NewFunc(s.Name.Pos(), pkg, name, nil) + hasTParamError := false // avoid duplicate type parameter errors + if s.Recv == nil { // regular function if name == "init" || name == "main" && pkg.name == "main" { - if d.TParamList != nil { - check.softErrorf(d, "func %s must have no type parameters", name) + if len(s.TParamList) != 0 { + check.softErrorf(s.TParamList[0], "func %s must have no type parameters", name) + hasTParamError = true } - if t := d.Type; len(t.ParamList) != 0 || len(t.ResultList) != 0 { - check.softErrorf(d, "func %s must have no arguments and no return values", name) + if t := s.Type; len(t.ParamList) != 0 || len(t.ResultList) != 0 { + check.softErrorf(s, "func %s must have no arguments and no return values", name) } } // don't declare init functions in the package scope - they are invisible if name == "init" { obj.parent = pkg.scope - check.recordDef(d.Name, obj) + check.recordDef(s.Name, obj) // init functions must have a body - if d.Body == nil { + if s.Body == nil { // TODO(gri) make this error message consistent with the others above check.softErrorf(obj.pos, "missing function body") } } else { - check.declare(pkg.scope, d.Name, obj, nopos) + check.declare(pkg.scope, s.Name, obj, nopos) } } else { // method // d.Recv != nil - if !acceptMethodTypeParams && len(d.TParamList) != 0 { + if !acceptMethodTypeParams && len(s.TParamList) != 0 { //check.error(d.TParamList.Pos(), invalidAST + "method must have no type parameters") - check.error(d, invalidAST+"method must have no type parameters") + check.error(s.TParamList[0], invalidAST+"method must have no type parameters") + hasTParamError = true } - ptr, recv, _ := check.unpackRecv(d.Recv.Type, false) + ptr, recv, _ := check.unpackRecv(s.Recv.Type, false) // (Methods with invalid receiver cannot be associated to a type, and // methods with blank _ names are never found; no need to collect any // of them. They will still be type-checked with all the other functions.) if recv != nil && name != "_" { methods = append(methods, methodInfo{obj, ptr, recv}) } - check.recordDef(d.Name, obj) + check.recordDef(s.Name, obj) + } + if len(s.TParamList) != 0 && !check.allowVersion(pkg, 1, 18) && !hasTParamError { + check.softErrorf(s.TParamList[0], "type parameters require go1.18 or later") } - info := &declInfo{file: fileScope, fdecl: d} + info := &declInfo{file: fileScope, fdecl: s} // Methods are not package-level objects but we still track them in the // object map so that we can handle them like regular functions (if the // receiver is invalid); also we need their fdecl info when associating diff --git a/src/cmd/compile/internal/types2/subst.go b/src/cmd/compile/internal/types2/subst.go index 918e5f3043..ff8dd13b6d 100644 --- a/src/cmd/compile/internal/types2/subst.go +++ b/src/cmd/compile/internal/types2/subst.go @@ -38,7 +38,7 @@ func (m substMap) lookup(tpar *TypeParam) Type { // subst returns the type typ with its type parameters tpars replaced by the // corresponding type arguments targs, recursively. subst doesn't modify the // incoming type. If a substitution took place, the result type is different -// from from the incoming type. +// from the incoming type. // // If the given typMap is non-nil, it is used in lieu of check.typMap. func (check *Checker) subst(pos syntax.Pos, typ Type, smap substMap, typMap map[string]*Named) Type { diff --git a/src/cmd/compile/internal/types2/testdata/check/decls0.src b/src/cmd/compile/internal/types2/testdata/check/decls0.src index f051a4f2ac..09e5d5c5ad 100644 --- a/src/cmd/compile/internal/types2/testdata/check/decls0.src +++ b/src/cmd/compile/internal/types2/testdata/check/decls0.src @@ -146,7 +146,7 @@ type ( m1(I5) } I6 interface { - S0 /* ERROR "not an interface" */ + S0 /* ERROR "non-interface type S0" */ } I7 interface { I1 diff --git a/src/cmd/compile/internal/types2/testdata/check/issues.src b/src/cmd/compile/internal/types2/testdata/check/issues.src index 692ed37ef4..d83a95af0e 100644 --- a/src/cmd/compile/internal/types2/testdata/check/issues.src +++ b/src/cmd/compile/internal/types2/testdata/check/issues.src @@ -79,11 +79,11 @@ func issue9473(a []int, b ...int) { // Check that embedding a non-interface type in an interface results in a good error message. func issue10979() { type _ interface { - int /* ERROR int is not an interface */ + int /* ERROR non-interface type int */ } type T struct{} type _ interface { - T /* ERROR T is not an interface */ + T /* ERROR non-interface type T */ } type _ interface { nosuchtype /* ERROR undeclared name: nosuchtype */ @@ -280,7 +280,7 @@ type issue25301b /* ERROR cycle */ = interface { } type issue25301c interface { - notE // ERROR struct\{\} is not an interface + notE // ERROR non-interface type struct\{\} } type notE = struct{} diff --git a/src/cmd/compile/internal/types2/testdata/check/main.go2 b/src/cmd/compile/internal/types2/testdata/check/main.go2 index b7ddeaa1a8..395e3bfec8 100644 --- a/src/cmd/compile/internal/types2/testdata/check/main.go2 +++ b/src/cmd/compile/internal/types2/testdata/check/main.go2 @@ -4,4 +4,4 @@ package main -func /* ERROR "func main must have no type parameters" */ main[T any]() {} +func main [T /* ERROR "func main must have no type parameters" */ any]() {} diff --git a/src/cmd/compile/internal/types2/testdata/check/typeparams.go2 b/src/cmd/compile/internal/types2/testdata/check/typeparams.go2 index 1ad80b1e1b..765d561f3b 100644 --- a/src/cmd/compile/internal/types2/testdata/check/typeparams.go2 +++ b/src/cmd/compile/internal/types2/testdata/check/typeparams.go2 @@ -304,8 +304,8 @@ var _ = f8[int, float64](0, 0, nil...) // test case for #18268 // init functions cannot have type parameters func init() {} -func init[/* ERROR func init must have no type parameters */ _ any]() {} -func init[/* ERROR func init must have no type parameters */ P any]() {} +func init[_ /* ERROR func init must have no type parameters */ any]() {} +func init[P /* ERROR func init must have no type parameters */ any]() {} type T struct {} diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue47818.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue47818.go2 new file mode 100644 index 0000000000..5334695b5e --- /dev/null +++ b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue47818.go2 @@ -0,0 +1,59 @@ +// Copyright 2021 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Parser accepts type parameters but the type checker +// needs to report any operations that are not permitted +// before Go 1.18. + +package go1_17 + +type T[P /* ERROR type parameters require go1\.18 or later */ any] struct{} + +// for init (and main, but we're not in package main) we should only get one error +func init[P /* ERROR func init must have no type parameters */ any]() {} +func main[P /* ERROR type parameters require go1\.18 or later */ any]() {} + +func f[P /* ERROR type parameters require go1\.18 or later */ any](x P) { + var _ T[ /* ERROR type instantiation requires go1\.18 or later */ int] + var _ (T[ /* ERROR type instantiation requires go1\.18 or later */ int]) + _ = T[ /* ERROR type instantiation requires go1\.18 or later */ int]{} + _ = T[ /* ERROR type instantiation requires go1\.18 or later */ int](struct{}{}) +} + +func (T[ /* ERROR type instantiation requires go1\.18 or later */ P]) g(x int) { + f[ /* ERROR function instantiation requires go1\.18 or later */ int](0) // explicit instantiation + (f[ /* ERROR function instantiation requires go1\.18 or later */ int])(0) // parentheses (different code path) + f( /* ERROR implicit function instantiation requires go1\.18 or later */ x) // implicit instantiation +} + +type C1 interface { + comparable // ERROR undeclared name: comparable \(requires version go1\.18 or later\) +} + +type C2 interface { + comparable // ERROR undeclared name: comparable \(requires version go1\.18 or later\) + int // ERROR embedding non-interface type int requires go1\.18 or later + ~ /* ERROR embedding interface element ~int requires go1\.18 or later */ int + int /* ERROR embedding interface element int\|~string requires go1\.18 or later */ | ~string +} + +type _ interface { + // errors for these were reported with their declaration + C1 + C2 +} + +type ( + _ comparable // ERROR undeclared name: comparable \(requires version go1\.18 or later\) + // errors for these were reported with their declaration + _ C1 + _ C2 + + _ = comparable // ERROR undeclared name: comparable \(requires version go1\.18 or later\) + // errors for these were reported with their declaration + _ = C1 + _ = C2 +) + +// TODO(gri) need test cases for imported constraint types (see also issue #47967) \ No newline at end of file diff --git a/src/cmd/compile/internal/types2/typeset.go b/src/cmd/compile/internal/types2/typeset.go index 14596b68a3..56f64ab405 100644 --- a/src/cmd/compile/internal/types2/typeset.go +++ b/src/cmd/compile/internal/types2/typeset.go @@ -271,6 +271,11 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_ switch u := under(typ).(type) { case *Interface: tset := computeInterfaceTypeSet(check, pos, u) + // If typ is local, an error was already reported where typ is specified/defined. + if check != nil && check.isImportedConstraint(typ) && !check.allowVersion(check.pkg, 1, 18) { + check.errorf(pos, "embedding constraint interface %s requires go1.18 or later", typ) + continue + } if tset.comparable { ityp.tset.comparable = true } @@ -279,6 +284,10 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_ } terms = tset.terms case *Union: + if check != nil && !check.allowVersion(check.pkg, 1, 18) { + check.errorf(pos, "embedding interface element %s requires go1.18 or later", u) + continue + } tset := computeUnionTypeSet(check, pos, u) if tset == &invalidTypeSet { continue // ignore invalid unions @@ -293,7 +302,7 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_ continue } if check != nil && !check.allowVersion(check.pkg, 1, 18) { - check.errorf(pos, "%s is not an interface", typ) + check.errorf(pos, "embedding non-interface type %s requires go1.18 or later", typ) continue } terms = termlist{{false, typ}} diff --git a/src/cmd/compile/internal/types2/typexpr.go b/src/cmd/compile/internal/types2/typexpr.go index 241c6d35fe..f3e415e4c7 100644 --- a/src/cmd/compile/internal/types2/typexpr.go +++ b/src/cmd/compile/internal/types2/typexpr.go @@ -38,14 +38,12 @@ func (check *Checker) ident(x *operand, e *syntax.Name, def *Named, wantType boo } return case universeAny, universeComparable: + // complain if necessary but keep going if !check.allowVersion(check.pkg, 1, 18) { - check.errorf(e, "undeclared name: %s (requires version go1.18 or later)", e.Value) - return - } - // If we allow "any" for general use, this if-statement can be removed (issue #33232). - if obj == universeAny { - check.error(e, "cannot use any outside constraint position") - return + check.softErrorf(e, "undeclared name: %s (requires version go1.18 or later)", e.Value) + } else if obj == universeAny { + // If we allow "any" for general use, this if-statement can be removed (issue #33232). + check.softErrorf(e, "cannot use any outside constraint position") } } check.recordUse(e, obj) @@ -274,6 +272,9 @@ func (check *Checker) typInternal(e0 syntax.Expr, def *Named) (T Type) { } case *syntax.IndexExpr: + if !check.allowVersion(check.pkg, 1, 18) { + check.softErrorf(e.Pos(), "type instantiation requires go1.18 or later") + } return check.instantiatedType(e.X, unpackExpr(e.Index), def) case *syntax.ParenExpr: diff --git a/test/fixedbugs/issue10975.go b/test/fixedbugs/issue10975.go index 876ea58ef9..a58ccce2db 100644 --- a/test/fixedbugs/issue10975.go +++ b/test/fixedbugs/issue10975.go @@ -10,7 +10,7 @@ package main type I interface { - int // ERROR "interface contains embedded non-interface|not an interface" + int // ERROR "interface contains embedded non-interface|embedding non-interface type" } func New() I { -- cgit v1.2.3-54-g00ecf From 0ac64f6d700b56fa793d9304bec621cf4dde6fd6 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 24 Aug 2021 20:20:07 -0700 Subject: cmd/compile/internal/types2: rename IsMethodSet to IsConstraint (cleanup) Invert the boolean result to match the new name. Change-Id: Ide6c649ed8ac3a5d263640309960e61a005c886e Reviewed-on: https://go-review.googlesource.com/c/go/+/344872 Trust: Robert Griesemer Trust: Dan Scales Reviewed-by: Dan Scales --- src/cmd/compile/internal/types2/interface.go | 2 +- src/cmd/compile/internal/types2/typeset.go | 6 ++---- src/cmd/compile/internal/types2/typexpr.go | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/cmd/compile/internal/types2/interface.go b/src/cmd/compile/internal/types2/interface.go index ccd3de0a6e..e57158d2d5 100644 --- a/src/cmd/compile/internal/types2/interface.go +++ b/src/cmd/compile/internal/types2/interface.go @@ -98,7 +98,7 @@ func (t *Interface) Empty() bool { return t.typeSet().IsAll() } func (t *Interface) IsComparable() bool { return t.typeSet().IsComparable() } // IsConstraint reports whether interface t is not just a method set. -func (t *Interface) IsConstraint() bool { return !t.typeSet().IsMethodSet() } +func (t *Interface) IsConstraint() bool { return t.typeSet().IsConstraint() } func (t *Interface) Underlying() Type { return t } func (t *Interface) String() string { return TypeString(t, nil) } diff --git a/src/cmd/compile/internal/types2/typeset.go b/src/cmd/compile/internal/types2/typeset.go index 56f64ab405..1673b9b4af 100644 --- a/src/cmd/compile/internal/types2/typeset.go +++ b/src/cmd/compile/internal/types2/typeset.go @@ -30,10 +30,8 @@ func (s *_TypeSet) IsAll() bool { return !s.comparable && len(s.methods) == 0 && s.terms.isAll() } -// TODO(gri) IsMethodSet is not a great name for this predicate. Find a better one. - -// IsMethodSet reports whether the type set s is described by a single set of methods. -func (s *_TypeSet) IsMethodSet() bool { return !s.comparable && s.terms.isAll() } +// IsConstraint reports whether type set s is not just a set of methods. +func (s *_TypeSet) IsConstraint() bool { return s.comparable || !s.terms.isAll() } // IsComparable reports whether each type in the set is comparable. func (s *_TypeSet) IsComparable() bool { diff --git a/src/cmd/compile/internal/types2/typexpr.go b/src/cmd/compile/internal/types2/typexpr.go index f3e415e4c7..6938648bbc 100644 --- a/src/cmd/compile/internal/types2/typexpr.go +++ b/src/cmd/compile/internal/types2/typexpr.go @@ -151,7 +151,7 @@ func (check *Checker) ordinaryType(pos syntax.Pos, typ Type) { check.later(func() { if t := asInterface(typ); t != nil { tset := computeInterfaceTypeSet(check, pos, t) // TODO(gri) is this the correct position? - if !tset.IsMethodSet() { + if tset.IsConstraint() { if tset.comparable { check.softErrorf(pos, "interface is (or embeds) comparable") } else { -- cgit v1.2.3-54-g00ecf From 4f2620285d7ce1802aff3d1f85e5ab0168d57bf3 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Tue, 24 Aug 2021 21:12:06 -0700 Subject: cmd/compile/internal/types2: fix type set printing and add test Change-Id: I44ca1f889b041467d5febacaf6037cfd75859175 Reviewed-on: https://go-review.googlesource.com/c/go/+/344873 Trust: Robert Griesemer Trust: Dan Scales Reviewed-by: Dan Scales --- src/cmd/compile/internal/types2/typeset.go | 12 ++--- src/cmd/compile/internal/types2/typeset_test.go | 67 ++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 8 deletions(-) diff --git a/src/cmd/compile/internal/types2/typeset.go b/src/cmd/compile/internal/types2/typeset.go index 1673b9b4af..ae39f26e4f 100644 --- a/src/cmd/compile/internal/types2/typeset.go +++ b/src/cmd/compile/internal/types2/typeset.go @@ -77,26 +77,24 @@ func (s *_TypeSet) String() string { var buf bytes.Buffer buf.WriteByte('{') if s.comparable { - buf.WriteString(" comparable") + buf.WriteString("comparable") if hasMethods || hasTerms { - buf.WriteByte(';') + buf.WriteString("; ") } } for i, m := range s.methods { if i > 0 { - buf.WriteByte(';') + buf.WriteString("; ") } - buf.WriteByte(' ') buf.WriteString(m.String()) } if hasMethods && hasTerms { - buf.WriteByte(';') + buf.WriteString("; ") } if hasTerms { buf.WriteString(s.terms.String()) } - buf.WriteString(" }") // there was at least one method or term - + buf.WriteString("}") return buf.String() } diff --git a/src/cmd/compile/internal/types2/typeset_test.go b/src/cmd/compile/internal/types2/typeset_test.go index 0e14d523c8..7f7cc06db9 100644 --- a/src/cmd/compile/internal/types2/typeset_test.go +++ b/src/cmd/compile/internal/types2/typeset_test.go @@ -4,7 +4,11 @@ package types2 -import "testing" +import ( + "cmd/compile/internal/syntax" + "strings" + "testing" +) func TestInvalidTypeSet(t *testing.T) { if !invalidTypeSet.IsEmpty() { @@ -12,4 +16,65 @@ func TestInvalidTypeSet(t *testing.T) { } } +func TestTypeSetString(t *testing.T) { + for body, want := range map[string]string{ + "{}": "𝓤", + "{int}": "{int}", + "{~int}": "{~int}", + "{int|string}": "{int ∪ string}", + "{int; string}": "∅", + + "{comparable}": "{comparable}", + "{comparable; int}": "{comparable; int}", + "{~int; comparable}": "{comparable; ~int}", + "{int|string; comparable}": "{comparable; int ∪ string}", + "{comparable; int; string}": "∅", + + "{m()}": "{func (p.T).m()}", + "{m1(); m2() int }": "{func (p.T).m1(); func (p.T).m2() int}", + "{error}": "{func (error).Error() string}", + "{m(); comparable}": "{comparable; func (p.T).m()}", + "{m1(); comparable; m2() int }": "{comparable; func (p.T).m1(); func (p.T).m2() int}", + "{comparable; error}": "{comparable; func (error).Error() string}", + + "{m(); comparable; int|float32|string}": "{comparable; func (p.T).m(); int ∪ float32 ∪ string}", + "{m1(); int; m2(); comparable }": "{comparable; func (p.T).m1(); func (p.T).m2(); int}", + + "{E}; type E interface{}": "𝓤", + "{E}; type E interface{int;string}": "∅", + "{E}; type E interface{comparable}": "{comparable}", + } { + // parse + errh := func(error) {} // dummy error handler so that parsing continues in presence of errors + src := "package p; type T interface" + body + file, err := syntax.Parse(nil, strings.NewReader(src), errh, nil, syntax.AllowGenerics) + if err != nil { + t.Fatalf("%s: %v (invalid test case)", body, err) + } + + // type check + var conf Config + pkg, err := conf.Check(file.PkgName.Value, []*syntax.File{file}, nil) + if err != nil { + t.Fatalf("%s: %v (invalid test case)", body, err) + } + + // lookup T + obj := pkg.scope.Lookup("T") + if obj == nil { + t.Fatalf("%s: T not found (invalid test case)", body) + } + T, ok := under(obj.Type()).(*Interface) + if !ok { + t.Fatalf("%s: %v is not an interface (invalid test case)", body, obj) + } + + // verify test case + got := T.typeSet().String() + if got != want { + t.Errorf("%s: got %s; want %s", body, got, want) + } + } +} + // TODO(gri) add more tests -- cgit v1.2.3-54-g00ecf From a6ff433d6a927e8ad8eaa6828127233296d12ce5 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Wed, 25 Aug 2021 14:56:01 +0700 Subject: cmd/go: pass -gcflags after other flags generated by the go command Otherwise, any gc flags set by user using "-gcflags" won't have effect. Fixes #47682 Change-Id: Icd365577cba1f64f6c7b8320d0c9019de9f062f6 Reviewed-on: https://go-review.googlesource.com/c/go/+/344909 Trust: Cuong Manh Le Run-TryBot: Cuong Manh Le TryBot-Result: Go Bot Reviewed-by: Jay Conrod --- src/cmd/go/internal/work/gc.go | 24 +++++++++++----------- src/cmd/go/testdata/script/build_gcflags_order.txt | 20 ++++++++++++++++++ .../go/testdata/script/build_runtime_gcflags.txt | 2 +- src/cmd/go/testdata/script/gcflags_patterns.txt | 22 ++++++++++---------- 4 files changed, 44 insertions(+), 24 deletions(-) create mode 100644 src/cmd/go/testdata/script/build_gcflags_order.txt diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go index eee8adca94..1cce5d4dd5 100644 --- a/src/cmd/go/internal/work/gc.go +++ b/src/cmd/go/internal/work/gc.go @@ -75,7 +75,7 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg } pkgpath := pkgPath(a) - gcargs := []string{"-p", pkgpath} + gcflags := []string{"-p", pkgpath} if p.Module != nil { v := p.Module.GoVersion if v == "" { @@ -94,11 +94,11 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg v = "1.16" } if allowedVersion(v) { - gcargs = append(gcargs, "-lang=go"+v) + gcflags = append(gcflags, "-lang=go"+v) } } if p.Standard { - gcargs = append(gcargs, "-std") + gcflags = append(gcflags, "-std") } _, compilingRuntime := runtimePackages[p.ImportPath] compilingRuntime = compilingRuntime && p.Standard @@ -106,7 +106,7 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg // runtime compiles with a special gc flag to check for // memory allocations that are invalid in the runtime package, // and to implement some special compiler pragmas. - gcargs = append(gcargs, "-+") + gcflags = append(gcflags, "-+") } // If we're giving the compiler the entire package (no C etc files), tell it that, @@ -125,25 +125,25 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg } } if extFiles == 0 { - gcargs = append(gcargs, "-complete") + gcflags = append(gcflags, "-complete") } if cfg.BuildContext.InstallSuffix != "" { - gcargs = append(gcargs, "-installsuffix", cfg.BuildContext.InstallSuffix) + gcflags = append(gcflags, "-installsuffix", cfg.BuildContext.InstallSuffix) } if a.buildID != "" { - gcargs = append(gcargs, "-buildid", a.buildID) + gcflags = append(gcflags, "-buildid", a.buildID) } if p.Internal.OmitDebug || cfg.Goos == "plan9" || cfg.Goarch == "wasm" { - gcargs = append(gcargs, "-dwarf=false") + gcflags = append(gcflags, "-dwarf=false") } if strings.HasPrefix(runtimeVersion, "go1") && !strings.Contains(os.Args[0], "go_bootstrap") { - gcargs = append(gcargs, "-goversion", runtimeVersion) + gcflags = append(gcflags, "-goversion", runtimeVersion) } if symabis != "" { - gcargs = append(gcargs, "-symabis", symabis) + gcflags = append(gcflags, "-symabis", symabis) } - gcflags := str.StringList(forcedGcflags, p.Internal.Gcflags) + gcflags = append(gcflags, str.StringList(forcedGcflags, p.Internal.Gcflags)...) if compilingRuntime { // Remove -N, if present. // It is not possible to build the runtime with no optimizations, @@ -157,7 +157,7 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg } } - args := []interface{}{cfg.BuildToolexec, base.Tool("compile"), "-o", ofile, "-trimpath", a.trimpath(), gcflags, gcargs} + args := []interface{}{cfg.BuildToolexec, base.Tool("compile"), "-o", ofile, "-trimpath", a.trimpath(), gcflags} if p.Internal.LocalPrefix != "" { // Workaround #43883. args = append(args, "-D", p.Internal.LocalPrefix) diff --git a/src/cmd/go/testdata/script/build_gcflags_order.txt b/src/cmd/go/testdata/script/build_gcflags_order.txt new file mode 100644 index 0000000000..0ffe1570f6 --- /dev/null +++ b/src/cmd/go/testdata/script/build_gcflags_order.txt @@ -0,0 +1,20 @@ +# Tests golang.org/issue/47682 +# Flags specified with -gcflags should appear after other flags generated by cmd/go. + +cd m +go build -n -gcflags=-lang=go1.17 +stderr ' -lang=go1.16.* -lang=go1.17' + +-- m/go.mod -- +module example.com + +go 1.16 + +-- m/main.go -- +package main + +func main() { + var s = []int{1, 2, 3} + var pa = (*[2]int)(s[1:]) + println(pa[1]) +} diff --git a/src/cmd/go/testdata/script/build_runtime_gcflags.txt b/src/cmd/go/testdata/script/build_runtime_gcflags.txt index da1b65f06c..c87e480911 100644 --- a/src/cmd/go/testdata/script/build_runtime_gcflags.txt +++ b/src/cmd/go/testdata/script/build_runtime_gcflags.txt @@ -8,4 +8,4 @@ mkdir $GOCACHE # Verify the standard library (specifically runtime/internal/atomic) can be # built with -gcflags when -n is given. See golang.org/issue/29346. go build -n -gcflags=all='-l' std -stderr 'compile.* -l .* runtime/internal/atomic' +stderr 'compile.* runtime/internal/atomic .* -l' diff --git a/src/cmd/go/testdata/script/gcflags_patterns.txt b/src/cmd/go/testdata/script/gcflags_patterns.txt index f23cecefd3..e9521c2fb2 100644 --- a/src/cmd/go/testdata/script/gcflags_patterns.txt +++ b/src/cmd/go/testdata/script/gcflags_patterns.txt @@ -7,28 +7,28 @@ env GOCACHE=$WORK/gocache # Looking for compile commands, so need a clean cache # -gcflags=-e applies to named packages, not dependencies go build -n -v -gcflags=-e z1 z2 -stderr 'compile.* -e.* -p z1' -stderr 'compile.* -e.* -p z2' +stderr 'compile.* -p z1.* -e' +stderr 'compile.* -p z2.* -e' stderr 'compile.* -p y' -! stderr 'compile.* -e.* -p [^z]' +! stderr 'compile.* -p [^z].* -e' # -gcflags can specify package=flags, and can be repeated; last match wins go build -n -v -gcflags=-e -gcflags=z1=-N z1 z2 -stderr 'compile.* -N.* -p z1' -! stderr 'compile.* -e.* -p z1' -! stderr 'compile.* -N.* -p z2' -stderr 'compile.* -e.* -p z2' +stderr 'compile.* -p z1.* -N' +! stderr 'compile.* -p z1.* -e' +! stderr 'compile.* -p z2.* -N' +stderr 'compile.* -p z2.* -e' stderr 'compile.* -p y' -! stderr 'compile.* -e.* -p [^z]' -! stderr 'compile.* -N.* -p [^z]' +! stderr 'compile.* -p [^z].* -e' +! stderr 'compile.* -p [^z].* -N' # -gcflags can have arbitrary spaces around the flags go build -n -v -gcflags=' z1 = -e ' z1 -stderr 'compile.* -e.* -p z1' +stderr 'compile.* -p z1.* -e' # -gcflags='all=-e' should apply to all packages, even with go test go test -c -n -gcflags='all=-e' z1 -stderr 'compile.* -e.* -p z3 ' +stderr 'compile.* -p z3.* -e ' # this particular -gcflags argument made the compiler crash ! go build -gcflags=-d=ssa/ z1 -- cgit v1.2.3-54-g00ecf From 770df2e18df01e64f8770301b0d3a5d6bfa04027 Mon Sep 17 00:00:00 2001 From: vinckr Date: Thu, 26 Aug 2021 10:59:02 +0000 Subject: crypto/tls: fix typo in PreferServerCipherSuites comment Fixing a typo, Deprected -> Deprecated. Change-Id: Ie0ccc9a57ae6a935b4f67154ac097dba4c3832ec GitHub-Last-Rev: 57337cc1bfa771111f229e7b899fdfdad3b1655e GitHub-Pull-Request: golang/go#47745 Reviewed-on: https://go-review.googlesource.com/c/go/+/342791 Trust: Dmitri Shuralyov Reviewed-by: Filippo Valsorda --- src/crypto/tls/common.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go index d561e61707..610a5162dd 100644 --- a/src/crypto/tls/common.go +++ b/src/crypto/tls/common.go @@ -659,7 +659,7 @@ type Config struct { // cipher suite based on logic that takes into account inferred client // hardware, server hardware, and security. // - // Deprected: PreferServerCipherSuites is ignored. + // Deprecated: PreferServerCipherSuites is ignored. PreferServerCipherSuites bool // SessionTicketsDisabled may be set to true to disable session ticket and -- cgit v1.2.3-54-g00ecf From d6bdae33e918f779e9e50c020d32042e569368e2 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 25 Aug 2021 18:13:28 -0700 Subject: cmd/compile/internal/types2: address some TODOs (cleanup) - Address some easy TODOs. - Remove some TODOs that are not correct anymore or are unimportent. - Simplify some code on the way. Change-Id: I4d20de3725b3a735022afe022cbc002b2798936d Reviewed-on: https://go-review.googlesource.com/c/go/+/345176 Trust: Robert Griesemer Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/types2/check_test.go | 3 -- src/cmd/compile/internal/types2/expr.go | 4 +- src/cmd/compile/internal/types2/hilbert_test.go | 3 +- src/cmd/compile/internal/types2/instantiate.go | 49 +++++++++------------- src/cmd/compile/internal/types2/named.go | 2 - src/cmd/compile/internal/types2/self_test.go | 7 +--- src/cmd/compile/internal/types2/signature.go | 7 +--- src/cmd/compile/internal/types2/stmt.go | 6 --- .../internal/types2/testdata/check/tinference.go2 | 6 +-- src/cmd/compile/internal/types2/tuple.go | 2 - src/cmd/compile/internal/types2/type.go | 1 - src/cmd/compile/internal/types2/typestring.go | 1 + src/cmd/compile/internal/types2/unify.go | 3 -- 13 files changed, 27 insertions(+), 67 deletions(-) diff --git a/src/cmd/compile/internal/types2/check_test.go b/src/cmd/compile/internal/types2/check_test.go index 41b0c54702..bc68e76407 100644 --- a/src/cmd/compile/internal/types2/check_test.go +++ b/src/cmd/compile/internal/types2/check_test.go @@ -20,9 +20,6 @@ // _ = x /* ERROR "not declared" */ + 1 // } -// TODO(gri) Also collect strict mode errors of the form /* STRICT ... */ -// and test against strict mode. - package types2_test import ( diff --git a/src/cmd/compile/internal/types2/expr.go b/src/cmd/compile/internal/types2/expr.go index d108093dac..86a8444ee2 100644 --- a/src/cmd/compile/internal/types2/expr.go +++ b/src/cmd/compile/internal/types2/expr.go @@ -127,9 +127,7 @@ func (check *Checker) overflow(x *operand) { } // opName returns the name of an operation, or the empty string. -// For now, only operations that might overflow are handled. -// TODO(gri) Expand this to a general mechanism giving names to -// nodes? +// Only operations that might overflow are handled. func opName(e *syntax.Operation) string { op := int(e.Op) if e.Y == nil { diff --git a/src/cmd/compile/internal/types2/hilbert_test.go b/src/cmd/compile/internal/types2/hilbert_test.go index 9f9dad6b64..03fea4fe7c 100644 --- a/src/cmd/compile/internal/types2/hilbert_test.go +++ b/src/cmd/compile/internal/types2/hilbert_test.go @@ -29,8 +29,7 @@ func TestHilbert(t *testing.T) { } // parse source - // TODO(gri) get rid of []bytes to string conversion below - f, err := parseSrc("hilbert.go", string(src)) + f, err := syntax.Parse(syntax.NewFileBase("hilbert.go"), bytes.NewReader(src), nil, nil, 0) if err != nil { t.Fatal(err) } diff --git a/src/cmd/compile/internal/types2/instantiate.go b/src/cmd/compile/internal/types2/instantiate.go index 8bea63ec86..b78ac3bea3 100644 --- a/src/cmd/compile/internal/types2/instantiate.go +++ b/src/cmd/compile/internal/types2/instantiate.go @@ -122,19 +122,17 @@ func (check *Checker) instantiate(pos syntax.Pos, typ Type, targs []Type, posLis // instance creates a type or function instance using the given original type // typ and arguments targs. For Named types the resulting instance will be // unexpanded. -func (check *Checker) instance(pos syntax.Pos, typ Type, targs []Type) (res Type) { - // TODO(gri) What is better here: work with TypeParams, or work with TypeNames? +func (check *Checker) instance(pos syntax.Pos, typ Type, targs []Type) Type { switch t := typ.(type) { case *Named: h := instantiatedHash(t, targs) if check != nil { - // typ may already have been instantiated with identical type arguments. In - // that case, re-use the existing instance. + // typ may already have been instantiated with identical type arguments. + // In that case, re-use the existing instance. if named := check.typMap[h]; named != nil { return named } } - tname := NewTypeName(pos, t.obj.pkg, t.obj.name, nil) named := check.newNamed(tname, t, nil, nil, nil) // methods and tparams are set when named is loaded named.targs = NewTypeList(targs) @@ -142,7 +140,8 @@ func (check *Checker) instance(pos syntax.Pos, typ Type, targs []Type) (res Type if check != nil { check.typMap[h] = named } - res = named + return named + case *Signature: tparams := t.TParams() if !check.validateTArgLen(pos, tparams.Len(), len(targs)) { @@ -151,30 +150,22 @@ func (check *Checker) instance(pos syntax.Pos, typ Type, targs []Type) (res Type if tparams.Len() == 0 { return typ // nothing to do (minor optimization) } - defer func() { - // If we had an unexpected failure somewhere don't panic below when - // asserting res.(*Signature). Check for *Signature in case Typ[Invalid] - // is returned. - if _, ok := res.(*Signature); !ok { - return - } - // If the signature doesn't use its type parameters, subst - // will not make a copy. In that case, make a copy now (so - // we can set tparams to nil w/o causing side-effects). - if t == res { - copy := *t - res = © - } - // After instantiating a generic signature, it is not generic - // anymore; we need to set tparams to nil. - res.(*Signature).tparams = nil - }() - res = check.subst(pos, typ, makeSubstMap(tparams.list(), targs), nil) - default: - // only types and functions can be generic - panic(fmt.Sprintf("%v: cannot instantiate %v", pos, typ)) + sig := check.subst(pos, typ, makeSubstMap(tparams.list(), targs), nil).(*Signature) + // If the signature doesn't use its type parameters, subst + // will not make a copy. In that case, make a copy now (so + // we can set tparams to nil w/o causing side-effects). + if sig == t { + copy := *sig + sig = © + } + // After instantiating a generic signature, it is not generic + // anymore; we need to set tparams to nil. + sig.tparams = nil + return sig } - return res + + // only types and functions can be generic + panic(fmt.Sprintf("%v: cannot instantiate %v", pos, typ)) } // validateTArgLen verifies that the length of targs and tparams matches, diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go index a3a2595a22..ccb1f265be 100644 --- a/src/cmd/compile/internal/types2/named.go +++ b/src/cmd/compile/internal/types2/named.go @@ -9,8 +9,6 @@ import ( "sync" ) -// TODO(gri) Clean up Named struct below; specifically the fromRHS field (can we use underlying?). - // A Named represents a named (defined) type. type Named struct { check *Checker diff --git a/src/cmd/compile/internal/types2/self_test.go b/src/cmd/compile/internal/types2/self_test.go index 4722fec988..e0d2e1b07a 100644 --- a/src/cmd/compile/internal/types2/self_test.go +++ b/src/cmd/compile/internal/types2/self_test.go @@ -24,12 +24,7 @@ func TestSelf(t *testing.T) { conf := Config{Importer: defaultImporter()} _, err = conf.Check("cmd/compile/internal/types2", files, nil) if err != nil { - // Importing go/constant doesn't work in the - // build dashboard environment. Don't report an error - // for now so that the build remains green. - // TODO(gri) fix this - t.Log(err) // replace w/ t.Fatal eventually - return + t.Fatal(err) } } diff --git a/src/cmd/compile/internal/types2/signature.go b/src/cmd/compile/internal/types2/signature.go index f1bf60ae8e..d28e7b8944 100644 --- a/src/cmd/compile/internal/types2/signature.go +++ b/src/cmd/compile/internal/types2/signature.go @@ -150,12 +150,7 @@ func (check *Checker) funcType(sig *Signature, recvPar *syntax.Field, tparams [] // bound is (possibly) parameterized in the context of the // receiver type declaration. Substitute parameters for the // current context. - // TODO(gri) should we assume now that bounds always exist? - // (no bound == empty interface) - if bound != nil { - bound = check.subst(tpar.obj.pos, bound, smap, nil) - tpar.bound = bound - } + tpar.bound = check.subst(tpar.obj.pos, bound, smap, nil) } } } diff --git a/src/cmd/compile/internal/types2/stmt.go b/src/cmd/compile/internal/types2/stmt.go index 7865c2d4f4..8cfdf92e67 100644 --- a/src/cmd/compile/internal/types2/stmt.go +++ b/src/cmd/compile/internal/types2/stmt.go @@ -52,11 +52,6 @@ func (check *Checker) funcBody(decl *declInfo, name string, sig *Signature, body check.error(body.Rbrace, "missing return") } - // TODO(gri) Should we make it an error to declare generic functions - // where the type parameters are not used? - // 12/19/2018: Probably not - it can make sense to have an API with - // all functions uniformly sharing the same type parameters. - // spec: "Implementation restriction: A compiler may make it illegal to // declare a variable inside a function body if the variable is never used." check.usage(sig.scope) @@ -422,7 +417,6 @@ func (check *Checker) stmt(ctxt stmtContext, s syntax.Stmt) { check.assignVar(lhs[0], &x) case *syntax.CallStmt: - // TODO(gri) get rid of this conversion to string kind := "go" if s.Tok == syntax.Defer { kind = "defer" diff --git a/src/cmd/compile/internal/types2/testdata/check/tinference.go2 b/src/cmd/compile/internal/types2/testdata/check/tinference.go2 index 0afb77c1e4..2409fef4ae 100644 --- a/src/cmd/compile/internal/types2/testdata/check/tinference.go2 +++ b/src/cmd/compile/internal/types2/testdata/check/tinference.go2 @@ -15,7 +15,7 @@ type any interface{} // f("a", "b", "c", "d") // f0("a", "b", "c", "d") // } -// +// // func f1[A any, B interface{~A}](A, B) // func _() { // f := f1[int] @@ -60,9 +60,7 @@ func _() { var _ string = x } -// TODO(gri) Need to flag invalid recursive constraints. At the -// moment these cause infinite recursions and stack overflow. -// func f7[A interface{type B}, B interface{~A}]() +func f7[A interface{*B}, B interface{~*A}]() {} // More realistic examples diff --git a/src/cmd/compile/internal/types2/tuple.go b/src/cmd/compile/internal/types2/tuple.go index a3946beab5..1356aae0b0 100644 --- a/src/cmd/compile/internal/types2/tuple.go +++ b/src/cmd/compile/internal/types2/tuple.go @@ -16,8 +16,6 @@ func NewTuple(x ...*Var) *Tuple { if len(x) > 0 { return &Tuple{vars: x} } - // TODO(gri) Don't represent empty tuples with a (*Tuple)(nil) pointer; - // it's too subtle and causes problems. return nil } diff --git a/src/cmd/compile/internal/types2/type.go b/src/cmd/compile/internal/types2/type.go index 4b8642aa96..ca5ecdc434 100644 --- a/src/cmd/compile/internal/types2/type.go +++ b/src/cmd/compile/internal/types2/type.go @@ -34,7 +34,6 @@ func (t *top) String() string { return TypeString(t, nil) } // under must only be called when a type is known // to be fully set up. func under(t Type) Type { - // TODO(gri) is this correct for *Union? if n := asNamed(t); n != nil { return n.under() } diff --git a/src/cmd/compile/internal/types2/typestring.go b/src/cmd/compile/internal/types2/typestring.go index 2c34d036db..9980408593 100644 --- a/src/cmd/compile/internal/types2/typestring.go +++ b/src/cmd/compile/internal/types2/typestring.go @@ -279,6 +279,7 @@ func writeTParamList(buf *bytes.Buffer, list []*TypeParam, qf Qualifier, visited func writeTypeName(buf *bytes.Buffer, obj *TypeName, qf Qualifier) { if obj == nil { + assert(instanceHashing == 0) // we need an object for instance hashing buf.WriteString("") return } diff --git a/src/cmd/compile/internal/types2/unify.go b/src/cmd/compile/internal/types2/unify.go index d4fbebc11b..72542e7d2e 100644 --- a/src/cmd/compile/internal/types2/unify.go +++ b/src/cmd/compile/internal/types2/unify.go @@ -433,9 +433,6 @@ func (u *unifier) nify(x, y Type, p *ifacePair) bool { xargs := x.targs.list() yargs := y.targs.list() - // TODO(gri) This is not always correct: two types may have the same names - // in the same package if one of them is nested in a function. - // Extremely unlikely but we need an always correct solution. if x.obj.pkg == y.obj.pkg && x.obj.name == y.obj.name { assert(len(xargs) == len(yargs)) for i, x := range xargs { -- cgit v1.2.3-54-g00ecf From 166b691b652356074ea346157e8bbc13933380aa Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 25 Aug 2021 21:48:21 -0700 Subject: cmd/compile/internal/types2: remove need for instance (struct) instance was only used to hold the instantiation position for lazy instantiation (and encode the fact that we have a lazy instantiation). Just use a (pointer to a) syntax.Pos instead. We could use a syntax.Pos (no pointer) and rely on the fact that we have a known position (or fake position, if need be) to indicate lazy instantiation. But using a pointer leads to a smaller Named struct. Change-Id: I441a839a125f453ad6c501de1ce499b72a2f67a4 Reviewed-on: https://go-review.googlesource.com/c/go/+/345177 Trust: Robert Griesemer Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/types2/instantiate.go | 2 +- src/cmd/compile/internal/types2/named.go | 18 +++++------------- src/cmd/compile/internal/types2/typestring.go | 2 +- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/cmd/compile/internal/types2/instantiate.go b/src/cmd/compile/internal/types2/instantiate.go index b78ac3bea3..f9cde24dfc 100644 --- a/src/cmd/compile/internal/types2/instantiate.go +++ b/src/cmd/compile/internal/types2/instantiate.go @@ -136,7 +136,7 @@ func (check *Checker) instance(pos syntax.Pos, typ Type, targs []Type) Type { tname := NewTypeName(pos, t.obj.pkg, t.obj.name, nil) named := check.newNamed(tname, t, nil, nil, nil) // methods and tparams are set when named is loaded named.targs = NewTypeList(targs) - named.instance = &instance{pos} + named.instPos = &pos if check != nil { check.typMap[h] = named } diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go index ccb1f265be..b4074aa3dc 100644 --- a/src/cmd/compile/internal/types2/named.go +++ b/src/cmd/compile/internal/types2/named.go @@ -17,7 +17,7 @@ type Named struct { orig *Named // original, uninstantiated type fromRHS Type // type (on RHS of declaration) this *Named type is derived from (for cycle reporting) underlying Type // possibly a *Named during setup; never a *Named once set up completely - instance *instance // position information for lazy instantiation, or nil + instPos *syntax.Pos // position information for lazy instantiation, or nil tparams *TParamList // type parameters, or nil targs *TypeList // type arguments (after instantiation), or nil methods []*Func // methods declared for this type (not the method set of this type); signatures are type-checked lazily @@ -240,24 +240,16 @@ func (n *Named) setUnderlying(typ Type) { } } -// instance holds position information for use in lazy instantiation. -// -// TODO(rfindley): instance is probably unnecessary now. See if it can be -// eliminated. -type instance struct { - pos syntax.Pos // position of type instantiation; for error reporting only -} - // expand ensures that the underlying type of n is instantiated. // The underlying type will be Typ[Invalid] if there was an error. func (n *Named) expand(typMap map[string]*Named) *Named { - if n.instance != nil { + if n.instPos != nil { // n must be loaded before instantiation, in order to have accurate // tparams. This is done implicitly by the call to n.TParams, but making it // explicit is harmless: load is idempotent. n.load() var u Type - if n.check.validateTArgLen(n.instance.pos, n.tparams.Len(), n.targs.Len()) { + if n.check.validateTArgLen(*n.instPos, n.tparams.Len(), n.targs.Len()) { if typMap == nil { if n.check != nil { typMap = n.check.typMap @@ -270,13 +262,13 @@ func (n *Named) expand(typMap map[string]*Named) *Named { typMap = map[string]*Named{h: n} } } - u = n.check.subst(n.instance.pos, n.orig.underlying, makeSubstMap(n.TParams().list(), n.targs.list()), typMap) + u = n.check.subst(*n.instPos, n.orig.underlying, makeSubstMap(n.TParams().list(), n.targs.list()), typMap) } else { u = Typ[Invalid] } n.underlying = u n.fromRHS = u - n.instance = nil + n.instPos = nil } return n } diff --git a/src/cmd/compile/internal/types2/typestring.go b/src/cmd/compile/internal/types2/typestring.go index 9980408593..1775fc6677 100644 --- a/src/cmd/compile/internal/types2/typestring.go +++ b/src/cmd/compile/internal/types2/typestring.go @@ -195,7 +195,7 @@ func writeType(buf *bytes.Buffer, typ Type, qf Qualifier, visited []Type) { } case *Named: - if t.instance != nil { + if t.instPos != nil { buf.WriteByte(instanceMarker) } writeTypeName(buf, t.obj, qf) -- cgit v1.2.3-54-g00ecf From 5e6a7e9b860d7c8f589eec3c123469ea8071689f Mon Sep 17 00:00:00 2001 From: Dan Kortschak Date: Mon, 23 Aug 2021 19:13:35 +0930 Subject: embed: remove reference to global variables in docs Refering to variable is both redundant since package scope is used, and incorrect since global variables are not described in the spec. Change-Id: Ib08a9f072fc800ee36549f758b68167d8f044878 Reviewed-on: https://go-review.googlesource.com/c/go/+/344214 Reviewed-by: Ian Lance Taylor Trust: Alexander Rakoczy Run-TryBot: Alexander Rakoczy TryBot-Result: Go Bot --- src/embed/embed.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/embed/embed.go b/src/embed/embed.go index 851cc216fc..5dcd7f227d 100644 --- a/src/embed/embed.go +++ b/src/embed/embed.go @@ -83,8 +83,7 @@ // // The //go:embed directive can be used with both exported and unexported variables, // depending on whether the package wants to make the data available to other packages. -// It can only be used with global variables at package scope, -// not with local variables. +// It can only be used with variables at package scope, not with local variables. // // Patterns must not match files outside the package's module, such as ‘.git/*’ or symbolic links. // Matches for empty directories are ignored. After that, each pattern in a //go:embed line -- cgit v1.2.3-54-g00ecf