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
go/types, types2: broken invariants with respect to generic receiver types #50619
Comments
Per virtual in-person discussion, I agree that we need to address this. The receiver type handling is a historical accident: before generics, the most expedient way to type-check a method signature was to treat the receiver parameter list just like any other parameter list. This didn't work anymore for parameterized method receiver types because they look like an instantiated type - yet they are really declaring their own type parameters. Rather than completely changing the original code, I added some pre-amble to extract and declare those type parameters beforehand, so that the receiver type could be type-checked as an instantiated type (and the rest of the code didn't need to change). That code was pretty simple originally, but over time has become more and more complex, with unintended side effects (like what we're observing now). In retrospect, it would have been better to treat receiver parameter lists specially from the start (at least if they have a generic receiver). |
Change https://golang.org/cl/380498 mentions this issue: |
Change https://golang.org/cl/380499 mentions this issue: |
Method signatures can introduce a significant number of edges into the type graph. One can imagine a generic type with many methods, each of which may use other instantiated types, etc. For performance, when type checking generic code, we should avoid unnecessary instantiation of methods wherever possible. This CL achieves this by making method instantiation lazy at the individual method level. It abstracts method access into a methodList type, which may be either eager or lazy. In the lazy case, methods are only instantiated when they are accessed via the Named.Method, MethodSet, or LookupFieldOrMethod APIs. Factoring out a methodList type makes it easier to verify that we're not leaking the methods slice anywhere, and as a side benefit reduces the size of *Named types in the case where there are no methods. The effective memory footprint of Named types with methods increases by a pointer (to hold the slice of guards), and the footprint of instantiated named types increases additionally by a sync.Once per method. We estimate that this memory increase is more than offset by the reduction in the number of instantiated methods. This also simplifies the code. Previously we had to work around the fact that named type expansion could occur before all signatures were set-up, by stashing the instantiated receiver into a partially filled-out *Func. With fully lazy methods, we can rely on the invariant that any use of methods in valid code can only occur after all signatures can be type checked. This means that we can fully instantiate the *Func, and don't need to deal with partially instantiated stubs. Finally, this CL fixes a bug (issue golang#50619), where traversing Method->Receiver Type->Method did not get us back where we started. This is fixed by not instantiating a new method if t is already the receiver base of the original method. A test is added to explicitly verify the invariant above, and more test cases are added for the behavior of Info with respect to generic code. Fixes golang#50619 Change-Id: I5b6d2bdc4404c9f5dcb583a29cb64e8af9794c54 Reviewed-on: https://go-review.googlesource.com/c/go/+/380499 Trust: Robert Findley <rfindley@google.com> Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
Our handling of receiver type on generic methods in go/types and types2 is confusing, and arguably incorrect.
While type checking method declarations, the type checker checks the receiver expression
T[P1,...PN]
, resulting in a receiver type that is instantiated with its receiver type parameters, and then assigns this receiver type to the signature being type-checked. This has two surprising consequences:Info.Instances
map, and its methods are subsequently re-substituted. This means that the defined methodF
(the thing inInfo.Defs
!) is not contained in the method set ofF.Type().(*types.Signature).Recv().Type()
!Of these, I think (1) is a perhaps peculiar choice that we must stick with, and (2) is a bug -- a consequence of our choice to treat the receiver as an instance and a later decision to instantiate methods on instances. I think this is probably not that difficult to fix: we should avoid naively type checking the receiver parameter list. It is certainly a release blocker to decide what to do here.
@griesemer, do you agree?
CC @timothy-king, who encountered this while working on go/ssa.
The text was updated successfully, but these errors were encountered: