Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: internal compiler error for new version of absdiff.go, failure during noding #50790

Closed
danscales opened this issue Jan 24, 2022 · 3 comments
Milestone

Comments

@danscales
Copy link
Contributor

Issue for Go 1.18 (generics)

I created a new version of the test test/typeparam/absdiff.go that works fully, now that we've disallowed using a type param on the right-hand-side of a type declaration. (Thanks to @perillo for posting a possible new implementation in #45639 that encouraged me to experiment with getting the test fully working again.)

With the new version, we get an assertion failure in typ0, caused while doing deferred processing of a function body.

Here's the new test case:

package main

import (
	"fmt"
	"math"
)

type Numeric interface {
	~int | ~int8 | ~int16 | ~int32 | ~int64 |
		~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 | ~uintptr |
		~float32 | ~float64 |
		~complex64 | ~complex128
}

// numericAbs matches a struct containing a numeric type that has an Abs method.
type numericAbs[T Numeric] interface {
	~struct{ Value T }
	Abs() 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}
	return dt.Abs()
}

// orderedNumeric matches numeric types that support the < operator.
type orderedNumeric interface {
	~int | ~int8 | ~int16 | ~int32 | ~int64 |
		~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 | ~uintptr |
		~float32 | ~float64
}

// Complex matches the two complex types, which do not have a < operator.
type Complex interface {
	~complex64 | ~complex128
}

// 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
}

func (a orderedAbs[T]) Abs() T {
	if a.Value < 0 {
		return -a.Value
	}
	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
}

func (a complexAbs[T]) Abs() T {
	r := float64(real(a.Value))
	i := float64(imag(a.Value))
	d := math.Sqrt(r*r + i*i)
	return T(complex(d, 0))
}

// 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 {
	return absDifference(orderedAbs[T]{a}, orderedAbs[T]{b})
}

// ComplexAbsDifference returns the absolute value of the difference
// between a and b, where a and b are of a complex type.
func ComplexAbsDifference[T Complex](a, b T) T {
	return absDifference(complexAbs[T]{a}, complexAbs[T]{b})
}

func main() {
	if got, want := OrderedAbsDifference(1.0, -2.0), 3.0; got != want {
		panic(fmt.Sprintf("got = %v, want = %v", got, want))
	}
	if got, want := OrderedAbsDifference(-1.0, 2.0), 3.0; got != want {
		panic(fmt.Sprintf("got = %v, want = %v", got, want))
	}
	if got, want := OrderedAbsDifference(-20, 15), 35; got != want {
		panic(fmt.Sprintf("got = %v, want = %v", got, want))
	}

	if got, want := ComplexAbsDifference(5.0+2.0i, 2.0-2.0i), 5+0i; got != want {
		panic(fmt.Sprintf("got = %v, want = %v", got, want))
	}
	if got, want := ComplexAbsDifference(2.0-2.0i, 5.0+2.0i), 5+0i; got != want {
		panic(fmt.Sprintf("got = %v, want = %v", got, want))
	}
}

I have a fix already, which I'll post immediately. We were not properly setting g.curDecl in the deferred function processing, so type params of generic functions were not being made as unique as they should be. This is definitely worth getting in for beta-2 (but not completely required if we miss the cutoff).

@randall77

@danscales danscales self-assigned this Jan 24, 2022
@danscales danscales added this to the Go1.18 milestone Jan 24, 2022
@gopherbot
Copy link

Change https://golang.org/cl/380594 mentions this issue: cmd/compile: new absdiff.go test, fix problem with g.curDecl

@perillo
Copy link
Contributor

perillo commented Jan 25, 2022

@danscales
There is a typo in the absDifference function documentation. AbsDifference should be absDifference.

@danscales
Copy link
Contributor Author

Thanks @perillo I will incorporate a fix to that typo into the next checkin that I need to make.

jproberts pushed a commit to jproberts/go that referenced this issue Jun 21, 2022
Added a new absdiff2.go test case, which works fully without using a
typeparam on the right-hand-side of a type declaration (which is
disallowed). Fixed an issue that the test revealed, which is that we
need to set g.curDecl properly for the "later" functions which are
deferred until after all declarations are initially processed. Also,
g.curDecl may be non-nil in typeDecl for local type declaration. So, we
adjust the associate assertion, and save/restore g.curDecl
appropriately.

Fixes golang#50790

Change-Id: Ieed76a7ad0a83bccb99cbad4bf98a7bfafbcbbd3
Reviewed-on: https://go-review.googlesource.com/c/go/+/380594
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants