cmd/compile/internal/types2: "real(0)" and "imag(0)" have type "untyped float", but value of kind constant.Int

[mdempsky]

On dev.typeparams, go tool compile -G=3 $(go env GOROOT)/test/fixedbugs/issue11945.go currently fails because the real(0) and imag(0) expressions have type "untyped float", but then their go/constant representation has kind constant.Int, whereas irgen expects it to be constant.Float.

My intuition is that the TypeAndValue.Value's constant.Kind should always match the TypeAndValue.Type's Underlying's BasicInfo's Is{Boolean,Integer,Float,Complex,String}.

However, if that expectation is incorrect/unreasonable, I think this would be reasonable to handle in irgen instead of types2. (With the recent types2 cleanup, untyped numeric values are only seen in constant declarations. We also already have the constant's type before we set the value, so it wouldn't be hard to coerce to the expected constant.Kind.)

/cc @griesemer @findleyr @danscales

[mdempsky]

FWIW, after CLs 286176, 286214, and 286215, this is the only remaining failure for "GO_GCFLAGS=-G=3 go run run.go". (Note though: errorcheck tests don't use GO_GCFLAGS.)

[mdempsky]

It looks like this may actually be a go/constant bug: the Go spec say the real and imag functions always return an untyped float when given an untyped constant argument, so I think constant.Real and constant.Imag should make the same guarantee.

[mdempsky]

Incidentally, I'm suspicious of the constant.ToFloat calls in go/types and types2's handling of complex (also copied to typecheck/const.go:makeComplex). I'd think 2i and complex(0, 2) should be interchangeable, yet the program below prints two different values (with or without -G=3).

package main

import "fmt"

const ulp1 = imag(1i + 2i / 3 - 5i / 3)
const ulp2 = imag(1i + complex(0, 2) / 3 - 5i / 3)

func main() {
	fmt.Printf("%g %g\n", ulp1, ulp2)
}

This discrepancy is because constant.ToFloat forces integer constants to big.Float representation, but constant.MakeImag (used to convert 2 into 2i in ulp1) does not.

[griesemer]

Thanks for tracking this down. There appear to be two separate issues at stake here:

1. Internally, go/constant is supposed to use the most "efficient" or accurate representation for a value of a specific kind. constant.ToFloat seems incorrect: The "default" representation for a constant.Float value is the rational representation, and if we change that in ToFloat, the above mentioned discrepancy disappears. So I think this is clearly a bug in ToFloat.

2. I believe originally, go/constant value kinds didn't have to match the type used by the type checker (go/types or types2). The kind simply described the value "as is". But I am not sure if this is still supposed to be true, or if we happen to have an implicit match between types and kinds at this point. That said, if I remove AssertValidTypeForConst(n.Type(), v) from ir/name.go:328 the test $(go env GOROOT)/test/fixedbugs/issue11945.go passes.

I need to think a bit more on both of these to convince myself of the correct solution.

[griesemer]

Factored out problem 1) into #43908.

[mdempsky]

That said, if I remove AssertValidTypeForConst(n.Type(), v) from ir/name.go:328 the test $(go env GOROOT)/test/fixedbugs/issue11945.go passes.

Without actually trying it, I think that's only because the declared constants aren't exported. If they were renamed from "_" to "ExportedConstant" or something, I expect it would cause another crash in the exporter, because the value representation wouldn't match the untyped kind.

I need to think a bit more on both of these to convince myself of the correct solution.

Ack. In the mean time, I'll look into adding a workaround/fix in irgen.constDecl, so we can enable GO_GCFLAGS=-G=3 go run run.go. In should be pretty minimal and easy to back out again if we decide to fix this within types2 instead.

[gopherbot]

Change https://golang.org/cl/286652 mentions this issue: [dev.typeparams] cmd/compile: force untyped constants from types2 to expected kind

[griesemer]

I can confirm that the exporter forces a particular constant representation based on type. However, go/types (and it's importers) do not expect this, nor was this the original intent for go/constant: The representation of a value can be independent of its type.

This was not an issue for the compiler before the use of go/constant for constant evaluation. But now that we use it, e.g. forcing an exported floating-point constant to "float" representation often will convert an exactly represented value (a fraction) into an arbitrary-precision float, causing a loss of precision.

I think we should try to avoid this, but it will require an extra byte of information (the constant.Kind) in exported values, and with that a version bump. We may want to consider adding the byte (only required for numerical values) when we extend the export format for generics. (We just need to make the space, we don't need to use the byte right away.)

[gopherbot]

Change https://golang.org/cl/292351 mentions this issue: go/internal/gcimporter: fix reexporting compiler data

[griesemer]

Confirming that go tool compile -G=3 $(go env GOROOT)/test/fixedbugs/issue11945.go now passes, so this is not an immediate issue anymore. The same is true when running this test through types2 and go/types.

The go/types constant representation is more flexible than what the compiler export data permits. Filed issue #45837 to cover that.