-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
proposal: go/types: export API for unification #63982
Comments
CC @griesemer @adonovan @mdempsky I think Robert and I had discussed this, at some point. This would be a rather invasive API. An initial, naive thought: I wonder how much of this can currently be achieved with e.g. I feel like over the past 6 months we've run into 3-4 different new APIs ideas that could almost (but not quite) be implemented with |
I think the API here could be made relatively clean if it simply takes two types, a map of type parameters to type arguments, and then unifies for type identity. Such an API would ignore type constraints or assignability, nor would it do any renaming. |
@griesemer interesting. I guess the question is whether this is an interface for unification, or inference. (nobody suggested an API for inference, but that is what I was thinking about). Unification is relatively clean, but inference is not. Is a unification API sufficient? It is for the use-case suggested by @dominikh. |
This came up during the tools call. If we can come up with a natural API that allows for future extension, I think we could add this for 1.23 or 1.24. For discussion, here's are two possible shapes for the API:
I think I'd prefer (1), as it makes it easier to perform multiple unifications in the same context, and allows for more future extension. For example, we may want to add a field on Unifier to be used for reporting more detailed errors. CC @adonovan |
IIUC the proposed use case of A good minimum bar to clear is "would we want such call edges in codesearch?" Unify might clear that bar.
It would be helpful to have other candidate usages. The current candidate usage works on a pair of types at a time: can I be a T? This would not need to share work if one asks at the right granularity. It also probably does not use typeMap. Would it be more helpful to give an error explaining why Unify failed if it does? Failures will be hard to explain outside of the type checker. Maybe explaining why a function cannot be called would be a helpful feature for an IDE? |
Just to make sure we're on the same page:
My use case (which is indeed related to the PR you've linked to) is similar to CHA, but operates entirely on types and doesn't depend on individual call sites. It aims to very conservatively answer whether methods of a given type can possibly contribute to an interface being implemented, while looking at a single package in isolation. What follows is a more explicit example and explanation, feel free to skip it if we're on the same page. To be more explicit: Given the code from my original comment, I would call This information is important to me because of this extended example:
The It is the generic variant of this case, which
Currently, for the non-generic version, Staticcheck reports
while for the generic version it incorrectly reports
The key difference between (We also consider |
I think we are on the same page. AFAIU the key difference with CHA and |
I've been implementing type inference for generics in gopls. Because no public API was available I had to vendor the internal unification library. I'm leaving a comment here as a future reminder to delete It should only require updating a couple lines in |
Supporting the LSP Implementations operation for generic types (#59224) requires unification too. I'm not yet convinced that a single Consider the question "might concrete type C implement interface I?": func f[T any]() {
type I interface { ... }
type C struct { embed[T]; ... }
...
} Within a given instantiation of this function, it is sufficient to consider only degenerate solutions in which the free type parameter T of both C and I is the same type. But it's possible that this function could be instantiated twice, such that values of |
Change https://go.dev/cl/634597 mentions this issue: |
Change https://go.dev/cl/634596 mentions this issue: |
@adonovan for better or worse (probably worse) the trick we used inside the type checker to avoid this problem is to 'rename' any type parameters that are bound (as opposed to free). Of course, that doesn't make for a good API. This needs more thought. We may need to logically separate "parameter" types from "argument" types in the API, breaking the symmetry. In that case |
It is unused and, in the one place we actually wanted to use it, it was the wrong tool for the job; what we need is unification. Updates golang/go#59224 Updates golang/go#63982 Change-Id: I05b6fe6f56e3f81f434e76398c20496950822bfb Reviewed-on: https://go-review.googlesource.com/c/tools/+/634597 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Findley <rfindley@google.com>
This CL adds support for generic types to the local (go/types-based) implementation of Implementations. Instead of using types.AssignableTo(C, I), which doesn't consider generics, we check that C has all the methods of I and that the signatures of each corresponding pair match when type parameters are allowed to stand for any type at all. A more precise implementation would use true unification, ensuring that only consistent substitutions are considered to match, but this is good enough for now. Perhaps when go/types adds a Unify API (#63982) this will be easier. See CL 634197 for the corresponding change to the global (fingerprint-based) implementation. Fixes golang/go#59224 Updates golang/go#63982 Change-Id: I6959f855b69436d52cafbe4de07bcf24b1e0f280 Reviewed-on: https://go-review.googlesource.com/c/tools/+/634596 Reviewed-by: Robert Findley <rfindley@google.com> Auto-Submit: Alan Donovan <adonovan@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
go/types
should offer API for type unification, to allow clients to unify types that the type-checker hasn't already unified while type-checking code.Staticcheck has a use for this as part of the
unused
check, to see if types implement possible instantiations of generic interfaces. For example, givenwe want to find out that
T1
implementsIface[int]
and thatT2
doesn't implement any possible instantiation ofIface
. For that, we would try to unifyT1.foo
andT1.bar
withIface.foo
andIface.bar
, under a shared mapping of type parameters, and similarly do the same forT2
.As such, the API should allow unifying multiple pairs of types using the same mapping of type parameters to types, report success/failure, and allow querying the mapping.
/cc @findleyr
The text was updated successfully, but these errors were encountered: