From b8248fab897da9bee2211a98df1656883ccecd6d Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 9 Mar 2022 10:01:24 -0800 Subject: [PATCH] go/types, types2: disable field accesses through type parameters This is a feature that is not understood well enough and may have subtle repercussions impacting future changes. Disable for Go 1.18. The actual change is trivial: disable a branch through a flag. The remaining changes are adjustments to tests. Fixes #51576. Change-Id: Ib77b038b846711a808315a8889b3904e72367bce Reviewed-on: https://go-review.googlesource.com/c/go/+/391135 Trust: Robert Griesemer Run-TryBot: Robert Griesemer Reviewed-by: Robert Findley TryBot-Result: Gopher Robot --- src/cmd/compile/internal/types2/lookup.go | 3 +- .../types2/testdata/fixedbugs/issue50417.go2 | 24 +++++++----- .../types2/testdata/fixedbugs/issue50782.go2 | 13 +++++-- src/go/types/lookup.go | 3 +- .../types/testdata/fixedbugs/issue50417.go2 | 24 +++++++----- .../types/testdata/fixedbugs/issue50782.go2 | 13 +++++-- test/typeparam/absdiff2.go | 32 +++++++++++----- test/typeparam/absdiffimp2.dir/a.go | 32 +++++++++++----- test/typeparam/issue50417.go | 6 +++ test/typeparam/issue50417b.go | 8 ++++ test/typeparam/issue50690a.go | 37 +++++++++++++------ test/typeparam/issue50690b.go | 26 +++++++++---- test/typeparam/issue50690c.go | 30 +++++++++++---- 13 files changed, 177 insertions(+), 74 deletions(-) diff --git a/src/cmd/compile/internal/types2/lookup.go b/src/cmd/compile/internal/types2/lookup.go index 0a2d2a5790..0832877226 100644 --- a/src/cmd/compile/internal/types2/lookup.go +++ b/src/cmd/compile/internal/types2/lookup.go @@ -70,7 +70,8 @@ func LookupFieldOrMethod(T Type, addressable bool, pkg *Package, name string) (o // see if there is a matching field (but not a method, those need to be declared // explicitly in the constraint). If the constraint is a named pointer type (see // above), we are ok here because only fields are accepted as results. - if obj == nil && isTypeParam(T) { + const enableTParamFieldLookup = false // see issue #51576 + if enableTParamFieldLookup && obj == nil && isTypeParam(T) { if t := coreType(T); t != nil { obj, index, indirect = lookupFieldOrMethod(t, addressable, pkg, name, false) if _, ok := obj.(*Var); !ok { diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50417.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50417.go2 index 50487fa2ff..2caef1b986 100644 --- a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50417.go2 +++ b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50417.go2 @@ -2,6 +2,10 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +// Field accesses through type parameters are disabled +// until we have a more thorough understanding of the +// implications on the spec. See issue #51576. + package p type Sf struct { @@ -9,13 +13,13 @@ type Sf struct { } func f0[P Sf](p P) { - _ = p.f - p.f = 0 + _ = p.f // ERROR p\.f undefined + p.f /* ERROR p\.f undefined */ = 0 } func f0t[P ~struct{f int}](p P) { - _ = p.f - p.f = 0 + _ = p.f // ERROR p\.f undefined + p.f /* ERROR p\.f undefined */ = 0 } var _ = f0[Sf] @@ -25,8 +29,8 @@ var _ = f0[Sm /* ERROR does not implement */ ] var _ = f0t[Sm /* ERROR does not implement */ ] func f1[P interface{ Sf; m() }](p P) { - _ = p.f - p.f = 0 + _ = p.f // ERROR p\.f undefined + p.f /* ERROR p\.f undefined */ = 0 p.m() } @@ -44,8 +48,8 @@ type Sfm struct { func (Sfm) m() {} func f2[P interface{ Sfm; m() }](p P) { - _ = p.f - p.f = 0 + _ = p.f // ERROR p\.f undefined + p.f /* ERROR p\.f undefined */ = 0 p.m() } @@ -56,8 +60,8 @@ var _ = f2[Sfm] type PSfm *Sfm func f3[P interface{ PSfm }](p P) { - _ = p.f - p.f = 0 + _ = p.f // ERROR p\.f undefined + p.f /* ERROR p\.f undefined */ = 0 p.m /* ERROR type P has no field or method m */ () } diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50782.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50782.go2 index 8f41b84163..fd1ab11b8c 100644 --- a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50782.go2 +++ b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue50782.go2 @@ -2,6 +2,10 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +// Field accesses through type parameters are disabled +// until we have a more thorough understanding of the +// implications on the spec. See issue #51576. + package p // The first example from the issue. @@ -18,9 +22,12 @@ type numericAbs[T Numeric] interface { // AbsDifference computes the absolute value of the difference of // a and b, where the absolute value is determined by the Abs method. func absDifference[T numericAbs[T /* ERROR T does not implement Numeric */]](a, b T) T { - // TODO: the error below should probably be positioned on the '-'. - d := a /* ERROR "invalid operation: operator - not defined" */ .Value - b.Value - return d.Abs() + // Field accesses are not permitted for now. Keep an error so + // we can find and fix this code once the situation changes. + return a.Value // ERROR a\.Value undefined + // TODO: The error below should probably be positioned on the '-'. + // d := a /* ERROR "invalid operation: operator - not defined" */ .Value - b.Value + // return d.Abs() } // The second example from the issue. diff --git a/src/go/types/lookup.go b/src/go/types/lookup.go index 501c230357..335fada7b7 100644 --- a/src/go/types/lookup.go +++ b/src/go/types/lookup.go @@ -70,7 +70,8 @@ func LookupFieldOrMethod(T Type, addressable bool, pkg *Package, name string) (o // see if there is a matching field (but not a method, those need to be declared // explicitly in the constraint). If the constraint is a named pointer type (see // above), we are ok here because only fields are accepted as results. - if obj == nil && isTypeParam(T) { + const enableTParamFieldLookup = false // see issue #51576 + if enableTParamFieldLookup && obj == nil && isTypeParam(T) { if t := coreType(T); t != nil { obj, index, indirect = lookupFieldOrMethod(t, addressable, pkg, name, false) if _, ok := obj.(*Var); !ok { diff --git a/src/go/types/testdata/fixedbugs/issue50417.go2 b/src/go/types/testdata/fixedbugs/issue50417.go2 index 50487fa2ff..2caef1b986 100644 --- a/src/go/types/testdata/fixedbugs/issue50417.go2 +++ b/src/go/types/testdata/fixedbugs/issue50417.go2 @@ -2,6 +2,10 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +// Field accesses through type parameters are disabled +// until we have a more thorough understanding of the +// implications on the spec. See issue #51576. + package p type Sf struct { @@ -9,13 +13,13 @@ type Sf struct { } func f0[P Sf](p P) { - _ = p.f - p.f = 0 + _ = p.f // ERROR p\.f undefined + p.f /* ERROR p\.f undefined */ = 0 } func f0t[P ~struct{f int}](p P) { - _ = p.f - p.f = 0 + _ = p.f // ERROR p\.f undefined + p.f /* ERROR p\.f undefined */ = 0 } var _ = f0[Sf] @@ -25,8 +29,8 @@ var _ = f0[Sm /* ERROR does not implement */ ] var _ = f0t[Sm /* ERROR does not implement */ ] func f1[P interface{ Sf; m() }](p P) { - _ = p.f - p.f = 0 + _ = p.f // ERROR p\.f undefined + p.f /* ERROR p\.f undefined */ = 0 p.m() } @@ -44,8 +48,8 @@ type Sfm struct { func (Sfm) m() {} func f2[P interface{ Sfm; m() }](p P) { - _ = p.f - p.f = 0 + _ = p.f // ERROR p\.f undefined + p.f /* ERROR p\.f undefined */ = 0 p.m() } @@ -56,8 +60,8 @@ var _ = f2[Sfm] type PSfm *Sfm func f3[P interface{ PSfm }](p P) { - _ = p.f - p.f = 0 + _ = p.f // ERROR p\.f undefined + p.f /* ERROR p\.f undefined */ = 0 p.m /* ERROR type P has no field or method m */ () } diff --git a/src/go/types/testdata/fixedbugs/issue50782.go2 b/src/go/types/testdata/fixedbugs/issue50782.go2 index 8f41b84163..fd1ab11b8c 100644 --- a/src/go/types/testdata/fixedbugs/issue50782.go2 +++ b/src/go/types/testdata/fixedbugs/issue50782.go2 @@ -2,6 +2,10 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +// Field accesses through type parameters are disabled +// until we have a more thorough understanding of the +// implications on the spec. See issue #51576. + package p // The first example from the issue. @@ -18,9 +22,12 @@ type numericAbs[T Numeric] interface { // AbsDifference computes the absolute value of the difference of // a and b, where the absolute value is determined by the Abs method. func absDifference[T numericAbs[T /* ERROR T does not implement Numeric */]](a, b T) T { - // TODO: the error below should probably be positioned on the '-'. - d := a /* ERROR "invalid operation: operator - not defined" */ .Value - b.Value - return d.Abs() + // Field accesses are not permitted for now. Keep an error so + // we can find and fix this code once the situation changes. + return a.Value // ERROR a\.Value undefined + // TODO: The error below should probably be positioned on the '-'. + // d := a /* ERROR "invalid operation: operator - not defined" */ .Value - b.Value + // return d.Abs() } // The second example from the issue. diff --git a/test/typeparam/absdiff2.go b/test/typeparam/absdiff2.go index f3e058d468..87a1ec6de1 100644 --- a/test/typeparam/absdiff2.go +++ b/test/typeparam/absdiff2.go @@ -23,15 +23,16 @@ type Numeric interface { // numericAbs matches a struct containing a numeric type that has an Abs method. type numericAbs[T Numeric] interface { - ~struct{ Value T } + ~struct{ Value_ T } Abs() T + Value() T } // absDifference computes the absolute value of the difference of // a and b, where the absolute value is determined by the Abs method. func absDifference[T Numeric, U numericAbs[T]](a, b U) T { - d := a.Value - b.Value - dt := U{Value: d} + d := a.Value() - b.Value() + dt := U{Value_: d} return dt.Abs() } @@ -50,20 +51,29 @@ type Complex interface { // orderedAbs is a helper type that defines an Abs method for // a struct containing an ordered numeric type. type orderedAbs[T orderedNumeric] struct { - Value T + Value_ T } func (a orderedAbs[T]) Abs() T { - if a.Value < 0 { - return -a.Value + if a.Value_ < 0 { + return -a.Value_ } - return a.Value + return a.Value_ +} + +// Field accesses through type parameters are disabled +// until we have a more thorough understanding of the +// implications on the spec. See issue #51576. +// Use accessor method instead. + +func (a orderedAbs[T]) Value() T { + return a.Value_ } // complexAbs is a helper type that defines an Abs method for // a struct containing a complex type. type complexAbs[T Complex] struct { - Value T + Value_ T } func realimag(x any) (re, im float64) { @@ -82,13 +92,17 @@ func realimag(x any) (re, im float64) { func (a complexAbs[T]) Abs() T { // TODO use direct conversion instead of realimag once #50937 is fixed - r, i := realimag(a.Value) + r, i := realimag(a.Value_) // r := float64(real(a.Value)) // i := float64(imag(a.Value)) d := math.Sqrt(r*r + i*i) return T(complex(d, 0)) } +func (a complexAbs[T]) Value() T { + return a.Value_ +} + // OrderedAbsDifference returns the absolute value of the difference // between a and b, where a and b are of an ordered type. func OrderedAbsDifference[T orderedNumeric](a, b T) T { diff --git a/test/typeparam/absdiffimp2.dir/a.go b/test/typeparam/absdiffimp2.dir/a.go index 43493e1430..dc64f2dcbe 100644 --- a/test/typeparam/absdiffimp2.dir/a.go +++ b/test/typeparam/absdiffimp2.dir/a.go @@ -17,15 +17,16 @@ type Numeric interface { // numericAbs matches a struct containing a numeric type that has an Abs method. type numericAbs[T Numeric] interface { - ~struct{ Value T } + ~struct{ Value_ T } Abs() T + Value() T } // absDifference computes the absolute value of the difference of // a and b, where the absolute value is determined by the Abs method. func absDifference[T Numeric, U numericAbs[T]](a, b U) T { - d := a.Value - b.Value - dt := U{Value: d} + d := a.Value() - b.Value() + dt := U{Value_: d} return dt.Abs() } @@ -44,20 +45,29 @@ type Complex interface { // orderedAbs is a helper type that defines an Abs method for // a struct containing an ordered numeric type. type orderedAbs[T orderedNumeric] struct { - Value T + Value_ T } func (a orderedAbs[T]) Abs() T { - if a.Value < 0 { - return -a.Value + if a.Value_ < 0 { + return -a.Value_ } - return a.Value + return a.Value_ +} + +// Field accesses through type parameters are disabled +// until we have a more thorough understanding of the +// implications on the spec. See issue #51576. +// Use accessor method instead. + +func (a orderedAbs[T]) Value() T { + return a.Value_ } // complexAbs is a helper type that defines an Abs method for // a struct containing a complex type. type complexAbs[T Complex] struct { - Value T + Value_ T } func realimag(x any) (re, im float64) { @@ -76,13 +86,17 @@ func realimag(x any) (re, im float64) { func (a complexAbs[T]) Abs() T { // TODO use direct conversion instead of realimag once #50937 is fixed - r, i := realimag(a.Value) + r, i := realimag(a.Value_) // r := float64(real(a.Value)) // i := float64(imag(a.Value)) d := math.Sqrt(r*r + i*i) return T(complex(d, 0)) } +func (a complexAbs[T]) Value() T { + return a.Value_ +} + // OrderedAbsDifference returns the absolute value of the difference // between a and b, where a and b are of an ordered type. func OrderedAbsDifference[T orderedNumeric](a, b T) T { diff --git a/test/typeparam/issue50417.go b/test/typeparam/issue50417.go index 3d5f2f2538..b32e270bdf 100644 --- a/test/typeparam/issue50417.go +++ b/test/typeparam/issue50417.go @@ -8,6 +8,11 @@ package main func main() {} +// Field accesses through type parameters are disabled +// until we have a more thorough understanding of the +// implications on the spec. See issue #51576. + +/* type Sf struct { f int } @@ -138,3 +143,4 @@ func f8[P Int4](p P) { } var _ = f8[*Sf] +*/ diff --git a/test/typeparam/issue50417b.go b/test/typeparam/issue50417b.go index 8c13a4ee36..1c803b09bd 100644 --- a/test/typeparam/issue50417b.go +++ b/test/typeparam/issue50417b.go @@ -6,6 +6,13 @@ package main +func main() {} + +// Field accesses through type parameters are disabled +// until we have a more thorough understanding of the +// implications on the spec. See issue #51576. + +/* import "fmt" type MyStruct struct { @@ -48,3 +55,4 @@ func main() { panic(fmt.Sprintf("got %d, want %d", got, want)) } } +*/ diff --git a/test/typeparam/issue50690a.go b/test/typeparam/issue50690a.go index 35e8c20e07..6691af0a07 100644 --- a/test/typeparam/issue50690a.go +++ b/test/typeparam/issue50690a.go @@ -29,34 +29,47 @@ func Sum[T Numeric](args ...T) T { // Ledger is an identifiable, financial record. type Ledger[T ~string, K Numeric] struct { - // ID identifies the ledger. - ID T + ID_ T // Amounts is a list of monies associated with this ledger. - Amounts []K + Amounts_ []K // SumFn is a function that can be used to sum the amounts // in this ledger. - SumFn func(...K) K + SumFn_ func(...K) K } +// Field accesses through type parameters are disabled +// until we have a more thorough understanding of the +// implications on the spec. See issue #51576. +// Use accessor methods instead. + +func (l Ledger[T, _]) ID() T { return l.ID_ } +func (l Ledger[_, K]) Amounts() []K { return l.Amounts_ } +func (l Ledger[_, K]) SumFn() func(...K) K { return l.SumFn_ } + func PrintLedger[ T ~string, K Numeric, - L ~struct { - ID T - Amounts []K - SumFn func(...K) K + L interface { + ~struct { + ID_ T + Amounts_ []K + SumFn_ func(...K) K + } + ID() T + Amounts() []K + SumFn() func(...K) K }, ](l L) { - fmt.Printf("%s has a sum of %v\n", l.ID, l.SumFn(l.Amounts...)) + fmt.Printf("%s has a sum of %v\n", l.ID(), l.SumFn()(l.Amounts()...)) } func main() { PrintLedger(Ledger[string, int]{ - ID: "fake", - Amounts: []int{1, 2, 3}, - SumFn: Sum[int], + ID_: "fake", + Amounts_: []int{1, 2, 3}, + SumFn_: Sum[int], }) } diff --git a/test/typeparam/issue50690b.go b/test/typeparam/issue50690b.go index 13e725ae0a..09c84e089d 100644 --- a/test/typeparam/issue50690b.go +++ b/test/typeparam/issue50690b.go @@ -18,24 +18,34 @@ func Print[T ~string](s T) { fmt.Println(s) } -func PrintWithPrinter[T ~string, S ~struct { - ID T - PrintFn func(T) +func PrintWithPrinter[T ~string, S interface { + ~struct { + ID T + PrintFn_ func(T) + } + PrintFn() func(T) }](message T, obj S) { - obj.PrintFn(message) + obj.PrintFn()(message) } type PrintShop[T ~string] struct { - ID T - PrintFn func(T) + ID T + PrintFn_ func(T) } +// Field accesses through type parameters are disabled +// until we have a more thorough understanding of the +// implications on the spec. See issue #51576. +// Use accessor method instead. + +func (s PrintShop[T]) PrintFn() func(T) { return s.PrintFn_ } + func main() { PrintWithPrinter( "Hello, world.", PrintShop[string]{ - ID: "fake", - PrintFn: Print[string], + ID: "fake", + PrintFn_: Print[string], }, ) } diff --git a/test/typeparam/issue50690c.go b/test/typeparam/issue50690c.go index 75e772cccd..2db1487ecb 100644 --- a/test/typeparam/issue50690c.go +++ b/test/typeparam/issue50690c.go @@ -18,19 +18,33 @@ func Print[T ~string](s T) { fmt.Println(s) } -func PrintWithPrinter[T ~string, S struct { - ID T - PrintFn func(T) +func PrintWithPrinter[T ~string, S interface { + ~struct { + ID T + PrintFn_ func(T) + } + PrintFn() func(T) }](message T, obj S) { - obj.PrintFn(message) + obj.PrintFn()(message) } func main() { PrintWithPrinter( "Hello, world.", - struct { - ID string - PrintFn func(string) - }{ID: "fake", PrintFn: Print[string]}, + StructWithPrinter{ID: "fake", PrintFn_: Print[string]}, ) } + +type StructWithPrinter struct { + ID string + PrintFn_ func(string) +} + +// Field accesses through type parameters are disabled +// until we have a more thorough understanding of the +// implications on the spec. See issue #51576. +// Use accessor method instead. + +func (s StructWithPrinter) PrintFn() func(string) { + return s.PrintFn_ +}