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
types2, go/types: repeated identical external instantiations of the same type will lead to different types #47103
Comments
Change https://golang.org/cl/333383 mentions this issue: |
This comment has been minimized.
This comment has been minimized.
Is this still an issue with the InstantiateLazy API? That already requires passing in the Checker as an argument, I think to allow deduplication. |
No, this is not an issue with InstantiateLazy, only with Instantiate which doesn't provide a *Checker or any other kind of context/environment. |
…must terminate (bug fix) When types2.Instantiate is called externally, no *Checker is provided and substitution doesn't have access to Checker.typMap; and instantiation of recursive generic types leads to an infinite recursion in subst. There was a local subster.cache but it was only set and never used. Replaced subster.cache with subster.typMap, which is set to the global Checker.typMap if available, and set to a local map otherwise. This prevents such infinite recursions. Added a simple test. More generally, because we don't have a global type map for external instantiations, instantiating the same type twice, independently but with the same type arguments, will result in two different types. This is not correct. We need to provide some form of context for external instantiations (which means the importers). This is a separate but related issue which is not yet addressed (filed #47103). Change-Id: I541556c677db54f7396fd0c88c7467894dfcf2e7 Reviewed-on: https://go-review.googlesource.com/c/go/+/333383 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Change https://golang.org/cl/333569 mentions this issue: |
…ate with Checker.Instantiate Allow Checker.Instantiate to work with a nil *Checker receiver (for now). This opens the door to passing in a *Checker at all times. Also, added a verify flag to Instantiate, InstantiateLazy, and instance, to be able to control if constraint satisfaction should be checked or not. Removed types2.Instantiate. For #47103. Change-Id: Ie00ce41b3e50a0fc4341e013922e5f874276d282 Reviewed-on: https://go-review.googlesource.com/c/go/+/333569 Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@findleyr Is there anything left to do here now that we have the global |
I do think we need more testing. Want to repurpose this issue, or shall I open a new one? |
@griesemer @findleyr Checking in as this is labeled a release-blocker for Go 1.18. Is there anything left on this issue? |
I think we need to also cover repeated instantiations of *Signature types, which are not currently handled. So this should remain as a release blocker. |
Change https://golang.org/cl/362801 mentions this issue: |
Extend the type checking context to allow de-duplicating *Signature instances, in addition to *Named instances. Naively we would deduplicate instances of different-but-identical origin *Signature types. That may be OK, but it seems a bit strange to get the same signature when instantiating two different functions. For now, differentiate *Signature types by prepending a unique identifier for the origin pointer, thus guaranteeing that instances de-duplicated if they come from the exact same (pointer identical) origin type. Updates #47103 Change-Id: I93cc3cacad195267fe0a5801f9c5a3b1e61eb907 Reviewed-on: https://go-review.googlesource.com/c/go/+/362801 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>
I am satisfied with this for 1.18. We may add additional testing during the freeze, but this is complete. EDIT: these changes have landed in go/types, and will be in types2 shortly. Our convention is to close the issue once the initial CLs land, as we have external means to track ports. |
Reminder issue:
Because we don't have a global type map for external
instantiations, instantiating the same type twice, independently but
with the same type arguments, will result in two different types. This
is not correct. We need to provide some form of context for external
instantiations (which means the importers). This is a separate issue,
not yet addressed.
cc: @findleyr, @danscales @mdempsky
The text was updated successfully, but these errors were encountered: