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: set the proper export version number before Go 1.18 #47654

Closed
danscales opened this issue Aug 11, 2021 · 10 comments
Closed

cmd/compile: set the proper export version number before Go 1.18 #47654

danscales opened this issue Aug 11, 2021 · 10 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@danscales
Copy link
Contributor

danscales commented Aug 11, 2021

When we get closer to the beta for 1.18, we need to set the export version number to its new value,
iexportVersionGo1_18 = 2.

We are temporarily setting the export version back to 1 (iexportVersionGoPosCol, also iexportVersionGoGenerics, since generic functions/types are backward compatible) for ease of internal testing, especially given a lot of x/tools and third-party tools that will break on the new export version.

@danscales danscales added this to the Go1.18 milestone Aug 11, 2021
@danscales danscales self-assigned this Aug 11, 2021
@danscales
Copy link
Contributor Author

https://go-review.googlesource.com/c/go/+/341211 is the change the needs to be mostly reverted to fix this issue.

gopherbot pushed a commit that referenced this issue Aug 11, 2021
This is a temporary change. We will revert this back before the 1.18
release. We make this change now to simplify testing, since a lot of
tools will break on the new export version.

Updates #47654.

Change-Id: I0650fa753bb11229c71254d779dd61b5c1af9cdf
Reviewed-on: https://go-review.googlesource.com/c/go/+/341211
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/341211 mentions this issue: [dev.typeparams] cmd/compile: change export version to 1.17 for testing

@seankhliao seankhliao added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 12, 2021
@gopherbot
Copy link

Change https://golang.org/cl/357049 mentions this issue: cmd/compile: update the export version for generics

gopherbot pushed a commit that referenced this issue Oct 25, 2021
Bump the export version to a new value iexportVersionGo1_18 (2). This
will give a better error message when old compilers/tools encounter the
new export format (that includes parameterized types and functions).

We are also making a breaking change in the format:
 - a 'kind' byte is added to constant values

Also updated tinter() to pass the implicit bit through during type
substitution.

Tested that all tests still pass if the iexportVersionCurrent is changed
back to 1 in typecheck/iexport.go, iimporter/iimport.go, and
gcimporter/iimport.go

Updates #47654

Change-Id: I1dbeb167a97f6c7e0b7e0c011d6bada5db312b36
Reviewed-on: https://go-review.googlesource.com/c/go/+/357049
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Dan Scales <danscales@google.com>
@danscales
Copy link
Contributor Author

@findleyr Is there anything more on your side about the export version that you need to do? If so, I can assign this bug to you as a reminder. Otherwise, I think we can just close this issue, right?

@findleyr
Copy link
Contributor

findleyr commented Nov 9, 2021

Yes, I think I still need to clean up once this has had enough time to propagate. Reassigning to myself, but also removing the release-blocker label.

@findleyr findleyr assigned findleyr and danscales and unassigned danscales Nov 9, 2021
@findleyr
Copy link
Contributor

findleyr commented Nov 9, 2021

The top comment here looks wrong:

When we get closer to the beta for 1.18, we need to set the export version number to its new value,
iexportVersionGenerics = 3.

We are temporarily setting the export version back to 2 (in dev.typeparams, soon to be merged to master) for ease of internal testing, especially given a lot of x/tools and third-party tools that will break on the new export version.

We set it back to 1, and the new value will be 2. @danscales maybe amend?

@danscales
Copy link
Contributor Author

OK, updated the top comment. And removed release-blocker.

As an update, we have already change the export version in the compiler and x/tools, but there are some cleanup items that @findleyr still needs to do. However, no longer a release blocker for beta1.

@danscales
Copy link
Contributor Author

@findleyr Can we close this now? I confirmed with @heschi that are internal testing has proceed along and went fine.

@findleyr
Copy link
Contributor

@danscales actually no, I just checked and there is still cleanup that needs to take place. Sending a CL now.

@gopherbot
Copy link

Change https://golang.org/cl/377554 mentions this issue: go/internal/gcimporter: set iexportVersionGenerics to 2

@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants