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
NeedsFix
Milestone

Comments

@danscales
Copy link
Contributor

@danscales 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

@danscales danscales commented Aug 11, 2021

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

@gopherbot gopherbot commented Aug 11, 2021

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 label Aug 12, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 19, 2021

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

@danscales danscales commented Nov 9, 2021

@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 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 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

@danscales danscales commented Nov 9, 2021

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

@danscales danscales commented Jan 10, 2022

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

@findleyr
Copy link
Contributor

@findleyr findleyr commented Jan 10, 2022

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

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 10, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix
Projects
None yet
Development

No branches or pull requests

4 participants