Skip to content

Commit

Permalink
go/types, types2: disable field accesses through type parameters
Browse files Browse the repository at this point in the history
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 <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
griesemer committed Mar 9, 2022
1 parent 7026eeb commit b8248fa
Show file tree
Hide file tree
Showing 13 changed files with 177 additions and 74 deletions.
3 changes: 2 additions & 1 deletion src/cmd/compile/internal/types2/lookup.go
Expand Up @@ -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 {
Expand Down
24 changes: 14 additions & 10 deletions src/cmd/compile/internal/types2/testdata/fixedbugs/issue50417.go2
Expand Up @@ -2,20 +2,24 @@
// 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 {
f int
}

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]
Expand All @@ -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()
}

Expand All @@ -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()
}

Expand All @@ -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 */ ()
}

Expand Down
13 changes: 10 additions & 3 deletions src/cmd/compile/internal/types2/testdata/fixedbugs/issue50782.go2
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion src/go/types/lookup.go
Expand Up @@ -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 {
Expand Down
24 changes: 14 additions & 10 deletions src/go/types/testdata/fixedbugs/issue50417.go2
Expand Up @@ -2,20 +2,24 @@
// 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 {
f int
}

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]
Expand All @@ -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()
}

Expand All @@ -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()
}

Expand All @@ -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 */ ()
}

Expand Down
13 changes: 10 additions & 3 deletions src/go/types/testdata/fixedbugs/issue50782.go2
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
32 changes: 23 additions & 9 deletions test/typeparam/absdiff2.go
Expand Up @@ -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()
}

Expand All @@ -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) {
Expand All @@ -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 {
Expand Down
32 changes: 23 additions & 9 deletions test/typeparam/absdiffimp2.dir/a.go
Expand Up @@ -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()
}

Expand All @@ -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) {
Expand All @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions test/typeparam/issue50417.go
Expand Up @@ -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
}
Expand Down Expand Up @@ -138,3 +143,4 @@ func f8[P Int4](p P) {
}
var _ = f8[*Sf]
*/
8 changes: 8 additions & 0 deletions test/typeparam/issue50417b.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -48,3 +55,4 @@ func main() {
panic(fmt.Sprintf("got %d, want %d", got, want))
}
}
*/

0 comments on commit b8248fa

Please sign in to comment.