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.

Sign up for GitHub

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

Jump to bottom

exportdata: support type parameterized aliases #68778

Closed
19 tasks done
timothy-king opened this issue Aug 7, 2024 · 19 comments
Closed
19 tasks done

exportdata: support type parameterized aliases #68778

timothy-king opened this issue Aug 7, 2024 · 19 comments
Assignees
@timothy-king
Labels
release-blocker
Milestone
Go1.24

Comments

@timothy-king
Copy link
Contributor

timothy-king commented Aug 7, 2024 •
edited
Loading

This issue is for tracking the work needed to enable type parameterized aliases (#46477) in the unified IR and 'i' export data formats.

Here are the know steps in an admissible order:

  • cmd/compile/internal/importer.ReadPackage sets pkgReader.enableAlias=true.
  • improve the x/tools ureader panic message to tell users to update if the unified IR version is too new.
  • Support more go module versions than go1.14 in cmd/internal/testdir.
  • {go/internal/gcimporter,cmd/compile/internal/importer}/iimport.go support 'B' tag for type aliases with type parameters.
  • Add a Version type to internal/pkgbits and documentation to better track version changes.
  • Expose version of bitstream via internal/pkgbits.PkgDecoder.version via {PkgDecoder,Decoder}.Version(). Store the current version being written and expose it via {PkgEncoder,Encoder}.Version().
  • Only read/write 'has init' and 'derived func info' fields in unified IR until version 1 in {go/internal/gcimporter,cmd/compile/internal/importer}/ureader.go and cmd/compile/internal/noder.
  • Only read/write 'derived info needed' field in unified IR until version 1 in {go/internal/gcimporter,cmd/compile/internal/importer}/ureader.go and cmd/compile/internal/noder.
  • Only read/write typeParamNames() in unified IR after version 2.
  • x/tools: copy iimport.go changes to support reading 'B' tag.
  • x/tools: update iexport.go changes to support writing 'B' tag.
  • x/tools: copy ureader.go changes for version 1 and version 2.
  • revendor x/tools in src/cmd for cmd/internal/moddeps.TestAllDependencies
  • x/tools: release a new minor version.
  • Change the version being written to 2 when the aliastypeparams GOEXPERIMENT is on.
  • Add optional test builds where GOEXPERIMENT=aliastypeparams=1 for compiler and x/tools.
  • Switch optional test builds to presubmit build where GOEXPERIMENT=aliastypeparams=1 for compiler and x/tools.
  • Add longtest postsubmit builds where GOEXPERIMENT=aliastypeparams=1 for compiler and x/tools.
  • Flip the aliastypeparams GOEXPERIMENT default value to true.

This will be updated as the plan updates.

@griesemer @dr2chase @findleyr @adonovan @mdempsky @cuonglm 
@timothy-king timothy-king added this to the Go1.24 milestone Aug 7, 2024
@timothy-king timothy-king self-assigned this Aug 7, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/604099 mentions this issue: cmd/compile/internal/importer: enable aliases

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/604102 mentions this issue: cmd/internal/testdir: add a -gomodversion flag

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/604635 mentions this issue: go/internal/gcimporter: indexed format imports for type parameters aliases

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/604596 mentions this issue: internal/pkgbits: improve ureader panic message

@cherrymui cherrymui mentioned this issue Aug 13, 2024
Open
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/605655 mentions this issue: internal/pkgbits: add Version type

gopherbot pushed a commit that referenced this issue Aug 15, 2024
@timothy-king
cmd/compile/internal/importer: enable aliases
209ed1a 
Flips the pkgReader.enableAlias flag to true when reading unified IR.
This was disabled while resolving #66873. This resolves the TODO to
flip it back to true.

Updates #66873
Updates #68778

Change-Id: Ifd52b0f9510d6bcf151de1c9a18d71ab548c14e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/604099
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/606035 mentions this issue: cmd/compile: deprecate has init and derived func instance

gopherbot pushed a commit to golang/tools that referenced this issue Aug 16, 2024
@timothy-king
internal/pkgbits: improve ureader panic message
66adacf 
Improves the ureader panic message to tell users what
package has the problem and to update their tools.

Updates golang/go#68778

Change-Id: Ia19f43aa2301627109abbebac0ddc7c5666ffd9a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/604596
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/606335 mentions this issue: internal/gcimporter: support type parameterized aliases in indexed format

gopherbot pushed a commit that referenced this issue Aug 16, 2024
@timothy-king
go/internal/gcimporter: indexed format imports for type parameters al…
660e7d6 
…iases

Add support for importing a new 'B' tag for type parameters aliases
in the indexed data format.

Updates #68778

Change-Id: I3bd82870d4c4619a3771de30baf6d54f6ee5959e
Reviewed-on: https://go-review.googlesource.com/c/go/+/604635
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
gopherbot pushed a commit that referenced this issue Aug 20, 2024
@timothy-king
cmd/internal/testdir: add a -gomodversion flag
ab41822 
Adds a -gomodversion flag to testdir. This sets the go version
in generated go.mod files. This is just runindir tests at the moment.
This is a building block so that tests can be written for exported
type parameterized aliases (like reproducing #68526).

This also adds a test that uses this feature. A type parameterized
alias is used so aliastypeparams and gotypesalias must be enabled.
gotypesalias is enabled by the go module version. The alias is not
exported and will not appear in exportdata. The test shows the
package containing the alias can be imported. This encapsulates
the level of support of type parameterized aliases in 1.23.

Updates #68526
Updates #68778

Change-Id: I8e20df6baa178e1d427d0fff627a16714d9c3b18
Reviewed-on: https://go-review.googlesource.com/c/go/+/604102
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Austin Clements <austin@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/607235 mentions this issue: cmd/compile/internal: write type parameters for aliases

gopherbot pushed a commit that referenced this issue Aug 20, 2024
@timothy-king
internal/pkgbits: add Version type
830621b 
Adds a new Version type to pkgbits to represent the version of the
bitstream. Versions let readers and writers know when different data is
expected to be present or not in the bitstream. These different pieces
of data are called Fields, as an analogy with fields of a struct.
Fields can be added, removed or changed in a Version. Extends Encoder
and Decoder to report which version they are.

Updates #68778

Change-Id: Iaffa1828544fb4cbc47a905de853449bc8e5b91f
Reviewed-on: https://go-review.googlesource.com/c/go/+/605655
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
gopherbot pushed a commit that referenced this issue Aug 20, 2024
@timothy-king
cmd/compile: deprecate has init and derived func instance
24fd1a0 
Removes 'has init' and 'derived func instance' fields from unified IR
starting with V2.

This should be a no-op at the moment as the writer is hardwired to create V1.

Updates #68778

Change-Id: I84a606cbc27cd6d8bd6eee2aff44c89f4aa7413c
Reviewed-on: https://go-review.googlesource.com/c/go/+/606035
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/607495 mentions this issue: internal/gcimporter: move indexed format docs

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/607535 mentions this issue: internal/aliases: add type parameters argument to NewAliases

gopherbot pushed a commit to golang/tools that referenced this issue Aug 21, 2024
@timothy-king
internal/gcimporter: move indexed format docs
9ef0547 
Move the documentation for indexed export data format from
  $GOROOT/src/cmd/compile/internal/typecheck/iexport.go
to
  x/tools/internal/gcimporter/iexport.go .
This is the only current writer of this format.

Updates golang/go#68778
Updates golang/go#68898

Change-Id: I365f56315d03affc5860cf407ea6e3d0caa1a88e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/607495
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/607498 mentions this issue: go/internal/gcimporter: parse materialized aliases

gopherbot pushed a commit to golang/tools that referenced this issue Aug 22, 2024
@timothy-king
internal/aliases: add type parameters argument to NewAliases
b5f24ec 
Adds a type parameters argument to NewAliases and updates all
usage locations. Also adds a unit test that creates a type
parameterized alias.

Updates golang/go#68778

Change-Id: I5e3e76a5f597cf658faa9036319eded33eeb9286
Reviewed-on: https://go-review.googlesource.com/c/tools/+/607535
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
gopherbot pushed a commit that referenced this issue Aug 23, 2024
@timothy-king
cmd/compile/internal: write type parameters for aliases
3b36d92 
Writes the field for type parameter names for aliases when
the bitstream is >= V2.

This is a no-op at the moment as the writer is hardwired to V1.

Updates #68778

Change-Id: I5887e3608239b9a6a47e3cc21cacb75b84e1d186
Reviewed-on: https://go-review.googlesource.com/c/go/+/607235
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
gopherbot pushed a commit to golang/tools that referenced this issue Aug 23, 2024
@timothy-king
internal/gcimporter: support type parameterized aliases in indexed fo…
0734f62 
…rmat

Adds support for type parameterized aliases in indexed format to gcimporter.

Updates golang/go#68778

Change-Id: I475ab30fee8d1d273f678496a1c2c12b011b8a95
Reviewed-on: https://go-review.googlesource.com/c/tools/+/606335
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/608215 mentions this issue: internal/pkgbits: add DerivedInfoNeeded

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/608216 mentions this issue: cmd/compile: deprecate derived info needed field

gopherbot pushed a commit that referenced this issue Aug 26, 2024
@cuonglm @gopherbot
internal/pkgbits: add DerivedInfoNeeded
c586dbb 
So next CL can use it to remove unnecessary derivedInfo needed field.

Updates #68778

Change-Id: Ia4e0f638beaf4a448fbf10a9aa1bc9425349a5e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/608215
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Tim King <taking@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
gopherbot pushed a commit that referenced this issue Aug 26, 2024
@cuonglm @gopherbot
cmd/compile: deprecate derived info needed field
5de9811 
This field is unused since shape-based stenciling was added for Unified
IR (CL 421821). The derived types information is now explicitly using
derived-type dictionaries (CL 331829).

This CL follows the pattern used in CL 606035.

Updates #68778

Change-Id: Ie784b6443c0a651854bfbcebb8a5166b1481408b
Reviewed-on: https://go-review.googlesource.com/c/go/+/608216
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Tim King <taking@google.com>
gopherbot pushed a commit that referenced this issue Aug 26, 2024
@timothy-king
go/internal/gcimporter: parse materialized aliases
bd3bb5a 
Parse materialized aliases in indexed format.

This was in https://go.dev/cl/574717 in x/tools.

Updates #68778

Change-Id: I2f0871aeb5a2e74c803176001f178757766a4a0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/607498
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/608595 mentions this issue: cmd/compile/internal/noder: write V2 bitstream aliastypeparams=1

gopherbot pushed a commit that referenced this issue Aug 28, 2024
@timothy-king
cmd/compile/internal/noder: write V2 bitstream aliastypeparams=1
9e8ea56 
Enables V2 unified IR bitstreams when GOEXPERIMENT aliastypeparams
are enabled.

Allows pkgbits.NewPkgEncoder to set the output version.
Reenables support for writing V0 streams.

Updates #68778
Updates #68526

Change-Id: I590c494d81ab7db148232ceaba52229068d1e986
Reviewed-on: https://go-review.googlesource.com/c/go/+/608595
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/609317 mentions this issue: internal/gcimporter: copy over ureader changes

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/609316 mentions this issue: internal/pkgbits: copy over changes for V2

@timothy-king timothy-king mentioned this issue Aug 30, 2024
Closed
gopherbot pushed a commit to golang/tools that referenced this issue Aug 30, 2024
@timothy-king
internal/pkgbits: copy over changes for V2
28f3bc0 
Copy over changes from GOROOT's internal/pkgbits for unified IR
version V2.

Updates golang/go#68778

Change-Id: I7813767e8c11a0d0227e2284af07ce0d86291476
Reviewed-on: https://go-review.googlesource.com/c/tools/+/609316
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
gopherbot pushed a commit to golang/tools that referenced this issue Sep 3, 2024
@timothy-king
internal/gcimporter: copy over ureader changes
2db563b 
Copy over the ureader.go changes from GOROOT's
go/internal/gcimporter.

Adds a test that goes through gc export data.

Updates golang/go#68778

Change-Id: Ie4b91dfdb1ab9f952631a34c3691dc84be8831a7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/609317
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/610036 mentions this issue: cmd: vendor golang.org/x/tools@2db563b

@timothy-king
Copy link
Contributor Author

Included in x/tools, 0.25.0 https://github.com/golang/tools/commits/v0.25.0/

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/613236 mentions this issue: all: enable alias type parameters GOEXPERIMENT by default

gopherbot pushed a commit that referenced this issue Sep 25, 2024
@timothy-king
all: enable alias type parameters GOEXPERIMENT by default
4a1167d 
For #68778

Change-Id: I4b39f84665262251ca014d3f5fe74b2fd434d51e
Reviewed-on: https://go-review.googlesource.com/c/go/+/613236
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Commit-Queue: Tim King <taking@google.com>
Reviewed-by: David Chase <drchase@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@timothy-king timothy-king

Labels
release-blocker
Projects
None yet
Milestone
Go1.24
Development

No branches or pull requests
2 participants
@timothy-king @gopherbot