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

Jump to bottom

cmd/compile: capture implicit constraints in export data #49040

Closed
findleyr opened this issue Oct 18, 2021 · 6 comments
Closed

cmd/compile: capture implicit constraints in export data #49040

findleyr opened this issue Oct 18, 2021 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone
Go1.18

Comments

@findleyr
Copy link
Contributor

With #48424, we allow users to elide interface{} in constraint expressions. We expose this in the type checker as func (*Interface) IsImplicit() bool. It would be nice to capture this in export data, so that we can preserve this implicit bit on import.

CC @mdempsky @griesemer
@findleyr findleyr mentioned this issue Oct 18, 2021
Closed
@findleyr findleyr added this to the Go1.18 milestone Oct 18, 2021
@findleyr findleyr added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Oct 18, 2021
@findleyr findleyr assigned findleyr and unassigned mdempsky Oct 20, 2021
@findleyr
Copy link
Contributor Author

I can do this. @griesemer and I discussed and this is probably as straightforward as exporting the embedded structural type, rather than the constraint interface itself, for implicit constraint interfaces. Then when importing, we can detect that the constraint is not an interface and wrap it in an implicit interface.

Prototyped in https://golang.org/cl/357109.

@gopherbot
Copy link

Change https://golang.org/cl/357796 mentions this issue: go/types, types2: add the Interface.MarkImplicit method

gopherbot pushed a commit that referenced this issue Oct 21, 2021
@findleyr
go/types, types2: add the Interface.MarkImplicit method
3beab0c 
Add a new interface method, MarkImplicit, to allow marking interfaces as
implicit from outside the type-checker. This is necessary so that we can
capture the implicit bit in export data, and use it from importers.

For #48424
For #49040

Change-Id: I999aba2a298f92432326d7ccbd87fe133a2e1a72
Reviewed-on: https://go-review.googlesource.com/c/go/+/357796
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
@findleyr
Copy link
Contributor Author

Update: the test failures in the prototype highlighted that trying to "cheat" and compute whether an interface is implicit based on the constraint type, is unclean. We've decided to opt for an explicit bit in the export format.

@gopherbot
Copy link

Change https://golang.org/cl/358034 mentions this issue: go/internal/gcimporter: add support for the Go 1.18 export data version

gopherbot pushed a commit to golang/tools that referenced this issue Oct 25, 2021
@findleyr
go/internal/gcimporter: add support for the Go 1.18 export data version
18096c5 
Add support for accepting the Go 1.18 export data version (2) to the
x/tools gcimporter, while preserving support for generic code at version
1.

For now, the exporter still outputs version 1.

For golang/go#49040

Change-Id: Ic4547e385ced72b88212d150bf16acaf200a010e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/358034
Trust: Robert Findley <rfindley@google.com>
Trust: Dan Scales <danscales@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: Dan Scales <danscales@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
@griesemer
Copy link
Contributor

@findleyr is there anything left to do here?

@findleyr
Copy link
Contributor Author

findleyr commented Nov 9, 2021

No, I don't think so. Closing.

@findleyr findleyr closed this as completed Nov 9, 2021
CarlosRosuero pushed a commit to CarlosRosuero/typeparams that referenced this issue Jun 13, 2023
@findleyr
go/internal/gcimporter: add support for the Go 1.18 export data version
ca70e9e 
Add support for accepting the Go 1.18 export data version (2) to the
x/tools gcimporter, while preserving support for generic code at version
1.

For now, the exporter still outputs version 1.

For golang/go#49040

Change-Id: Ic4547e385ced72b88212d150bf16acaf200a010e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/358034
Trust: Robert Findley <rfindley@google.com>
Trust: Dan Scales <danscales@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: Dan Scales <danscales@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
@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.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
4 participants
@mdempsky @griesemer @gopherbot @findleyr