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: export format of untyped constants is unnecessarily restricted #45837

Open
griesemer opened this issue Apr 29, 2021 · 3 comments
Open
Assignees
Labels
Milestone

Comments

@griesemer
Copy link
Contributor

@griesemer griesemer commented Apr 29, 2021

The compiler exports untyped constants' format to their respective type. types2 and go/types together with the go/constant package don't require a constant representation to match the (untyped) constant type. For instance, it's ok to represent real(0) of type untyped float with a constant of kind constant.Int.

Should remove this restriction from the export format as it may cause loss of precision and also lead to larger than necessary export data. This will require an extra byte for each exported constant, encoding the respective constant kind. This will also require a bump of the export data version.

Marking for go1.18 since we are changing the export format anyway (generics). But it's not crucial to get this in.

See #43891 for details.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Apr 29, 2021

Does go/types only ever use more precise representations, or can it use more relaxed representations too? E.g., can an untyped integer/rune constant be represented by a constant.Float, or an untyped float represented by a constant.Complex?

The export data format should also be extended to support rational constants, so they don't need to be truncated to floating point. (We've discussed this before, but I don't see any open issues about it.)

Loading

@griesemer
Copy link
Contributor Author

@griesemer griesemer commented Apr 30, 2021

I don't think that the representation can be more relaxed. Untyped constants's kind usually matches the constant kind but when we apply a type (e.g. an untyped 1 is really a float64 1.0) in context we don't necessarily change the constant representation, leading to a 1.0 that is represented as an constant.Int 1.

Agreed on the rational constants.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 22, 2021

Change https://golang.org/cl/358035 mentions this issue: go/internal/gcimporter: stub support importing/exporting constant kind

Loading

gopherbot pushed a commit to golang/tools that referenced this issue Oct 25, 2021
Add placeholder support for importing/exporting constant kind in the Go
1.18 export data format.

Also eliminate the redundant iimport.iexportVersion field.

For golang/go#45837

Change-Id: I94dbcadde0fae55788ce1a139a2760276f644630
Reviewed-on: https://go-review.googlesource.com/c/tools/+/358035
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants