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(-) (limited to 'src/cmd/compile') 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 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 (limited to 'src/cmd/compile') 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 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 (limited to 'src/cmd/compile') 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 (limited to 'src/cmd/compile') 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 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(-) (limited to 'src/cmd/compile') 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 (limited to 'src/cmd/compile') 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 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(+) (limited to 'src/cmd/compile') 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 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(-) (limited to 'src/cmd/compile') 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(-) (limited to 'src/cmd/compile') 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 (limited to 'src/cmd/compile') 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(-) (limited to 'src/cmd/compile') 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(-) (limited to 'src/cmd/compile') 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 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(-) (limited to 'src/cmd/compile') 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(-) (limited to 'src/cmd/compile') 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