]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: fix unified IR's pointer-shaping
authorMatthew Dempsky <mdempsky@google.com>
Fri, 19 Aug 2022 03:59:26 +0000 (20:59 -0700)
committerMatthew Dempsky <mdempsky@google.com>
Tue, 23 Aug 2022 18:14:10 +0000 (18:14 +0000)
In CL 424734, I implemented pointer shaping for unified IR. Evidently
though, we didn't have any test cases that check that uses of
pointer-shaped expressions were handled correctly.

In the reported test case, the struct field "children items[*node[T]]"
gets shaped to "children items[go.shape.*uint8]" (underlying type
"[]go.shape.*uint8"); and so the expression "n.children[i]" has type
"go.shape.*uint8" and the ".items" field selection expression fails.

The fix implemented in this CL is that any expression of derived type
now gets an explicit "reshape" operation applied to it, to ensure it
has the appropriate type for its context. E.g., the "n.children[i]"
OINDEX expression above gets "reshaped" from "go.shape.*uint8" to
"*node[go.shape.int]", allowing the field selection to succeed.

This CL also adds a "-d=reshape" compiler debugging flag, because I
anticipate debugging reshaping operations will be something to come up
again in the future.

Fixes #54535.

Change-Id: Id847bd8f51300d2491d679505ee4d2e974ca972a
Reviewed-on: https://go-review.googlesource.com/c/go/+/424936
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: hopehook <hopehook@qq.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/compile/internal/base/debug.go
src/cmd/compile/internal/noder/codes.go
src/cmd/compile/internal/noder/reader.go
src/cmd/compile/internal/noder/writer.go
test/typeparam/issue54535.go [new file with mode: 0644]

index f1d020f342ed0f3126f5c06b251cac077063c0ae..5edb665e37bccc30ceeb2f0bd37176d8da4b2ea7 100644 (file)
@@ -34,6 +34,7 @@ type DebugFlags struct {
        NoRefName            int    `help:"do not include referenced symbol names in object file"`
        PCTab                string `help:"print named pc-value table\nOne of: pctospadj, pctofile, pctoline, pctoinline, pctopcdata"`
        Panic                int    `help:"show all compiler panics"`
+       Reshape              int    `help:"print information about expression reshaping"`
        Slice                int    `help:"print information about slice compilation"`
        SoftFloat            int    `help:"force compiler to emit soft-float code"`
        SyncFrames           int    `help:"how many writer stack frames to include at sync points in unified export data"`
index fb4fb4a8865ee32aa2911e5560b0084ac0cabed0..c1ee8d15c5d15b0b55310d0e28a17550cf8ac6ab 100644 (file)
@@ -58,6 +58,7 @@ const (
        exprNil
        exprFuncInst
        exprRecv
+       exprReshape
 )
 
 type codeAssign int
index 5f770166dbc1ad2ad65e164a3320820566b46d24..cf1e1440df000b2c4d27504acefcfb85bc3e2e59 100644 (file)
@@ -89,7 +89,7 @@ func (pr *pkgReader) newReader(k pkgbits.RelocKind, idx pkgbits.Index, marker pk
        }
 }
 
-// A writer provides APIs for reading an individual element.
+// A reader provides APIs for reading an individual element.
 type reader struct {
        pkgbits.Decoder
 
@@ -2367,6 +2367,41 @@ func (r *reader) expr() (res ir.Node) {
                typ := r.exprType()
                return typecheck.Expr(ir.NewUnaryExpr(pos, ir.ONEW, typ))
 
+       case exprReshape:
+               typ := r.typ()
+               x := r.expr()
+
+               if types.IdenticalStrict(x.Type(), typ) {
+                       return x
+               }
+
+               // Comparison expressions are constructed as "untyped bool" still.
+               //
+               // TODO(mdempsky): It should be safe to reshape them here too, but
+               // maybe it's better to construct them with the proper type
+               // instead.
+               if x.Type() == types.UntypedBool && typ.IsBoolean() {
+                       return x
+               }
+
+               base.AssertfAt(x.Type().HasShape() || typ.HasShape(), x.Pos(), "%L and %v are not shape types", x, typ)
+               base.AssertfAt(types.Identical(x.Type(), typ), x.Pos(), "%L is not shape-identical to %v", x, typ)
+
+               // We use ir.HasUniquePos here as a check that x only appears once
+               // in the AST, so it's okay for us to call SetType without
+               // breaking any other uses of it.
+               //
+               // Notably, any ONAMEs should already have the exactly right shape
+               // type and been caught by types.IdenticalStrict above.
+               base.AssertfAt(ir.HasUniquePos(x), x.Pos(), "cannot call SetType(%v) on %L", typ, x)
+
+               if base.Debug.Reshape != 0 {
+                       base.WarnfAt(x.Pos(), "reshaping %L to %v", x, typ)
+               }
+
+               x.SetType(typ)
+               return x
+
        case exprConvert:
                implicit := r.Bool()
                typ := r.typ()
index c2c3567220a1569528ccc32cdb4a5877b00577d7..75ff000249f44d8dbda2e628bc44bc46d3424383 100644 (file)
@@ -1629,6 +1629,16 @@ func (w *writer) expr(expr syntax.Expr) {
                        w.typ(tv.Type)
                        return
                }
+
+               // With shape types (and particular pointer shaping), we may have
+               // an expression of type "go.shape.*uint8", but need to reshape it
+               // to another shape-identical type to allow use in field
+               // selection, indexing, etc.
+               if typ := tv.Type; !tv.IsBuiltin() && !isTuple(typ) && !isUntyped(typ) {
+                       w.Code(exprReshape)
+                       w.typ(typ)
+                       // fallthrough
+               }
        }
 
        if obj != nil {
@@ -2199,6 +2209,11 @@ func isUntyped(typ types2.Type) bool {
        return ok && basic.Info()&types2.IsUntyped != 0
 }
 
+func isTuple(typ types2.Type) bool {
+       _, ok := typ.(*types2.Tuple)
+       return ok
+}
+
 func (w *writer) itab(typ, iface types2.Type) {
        typ = types2.Default(typ)
        iface = types2.Default(iface)
diff --git a/test/typeparam/issue54535.go b/test/typeparam/issue54535.go
new file mode 100644 (file)
index 0000000..574b275
--- /dev/null
@@ -0,0 +1,37 @@
+// run
+
+// Copyright 2022 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 node[T any] struct {
+       items    items[T]
+       children items[*node[T]]
+}
+
+func (n *node[T]) f(i int, j int) bool {
+       if len(n.children[i].items) < j {
+               return false
+       }
+       return true
+}
+
+type items[T any] []T
+
+func main() {
+       _ = node[int]{}
+       _ = f[int]
+}
+
+type s[T, U any] struct {
+       a T
+       c U
+}
+
+func f[T any]() {
+       var x s[*struct{ b T }, *struct{ d int }]
+       _ = x.a.b
+       _ = x.c.d
+}