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

go/types, types2: consider only calling under at top-level in unification #59750

Closed
griesemer opened this issue Apr 21, 2023 · 3 comments
Closed
Assignees
Labels
generics Issue is related to generics NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. TypeInference Issue is related to generic type inference
Milestone

Comments

@griesemer
Copy link
Contributor

This is a follow-up on #59740.

Per e-mail from @findleyr :


I think these types of problems can only arise when we allow taking 'under' when we shouldn't. Consider the abbreviated sequence below.

That 'under' in step 4 is not actually valid. The function types of step 1 are only assignable to each other if they are identical, at which point the 'under' transformation should be invalid.

I think the trick of taking 'under' is just a lossy encoding of the assignability condition for function argument type inference, and should not be a part of the core unification step.

  1. func(p.F[int]) ≡ func(func(p.F[T₃]))
  2. => (p.F[int]) ≡ (func(p.F[T₃]))
  3. => p.F[int] ≡ func(p.F[T₃])
  4. => func(func(p.F[int])) ≡ func(p.F[T₃]) (under)

Investigation: a quick change to the code shows:

  1. If we only call "under" at the top level this case fails with an error message (as predicted):
    type func(F[int]) of f does not match F[T] (cannot infer T)

  2. Some existing tests fail, too, though just about a dozen or so. I've done a cursory examination and it might be possible to address them by being slightly more careful about the under calls (e.g., also allow them at the top-level after looking up a type parameter value).

  3. Simply not panicking (but silently return) when we reach the recursion stack limit (i.e., setting panicAtUnificationDepthLimit to false) produces the same error:
    type func(F[int]) of f does not match F[T] (cannot infer T)

We should investigate if we can solve #59740 more correctly by using a more careful approach to calling under.

@griesemer griesemer added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. generics Issue is related to generics TypeInference Issue is related to generic type inference labels Apr 21, 2023
@griesemer griesemer added this to the Go1.22 milestone Apr 21, 2023
@griesemer griesemer self-assigned this Apr 21, 2023
@gopherbot
Copy link

Change https://go.dev/cl/487197 mentions this issue: go/types, types2: abort type unification if no progress is made

gopherbot pushed a commit that referenced this issue Apr 21, 2023
Fixes #59740.
For #59750.

Change-Id: I153d0a412bdfb15f81d6999e29691dc093fd0fcb
Reviewed-on: https://go-review.googlesource.com/c/go/+/487197
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
@griesemer
Copy link
Contributor Author

This issue was addressed with CL 498895 which uses exact unification for element (component) types in assignments.

@gopherbot
Copy link

Change https://go.dev/cl/499282 mentions this issue: doc/go1.21: document type inference changes

gopherbot pushed a commit that referenced this issue May 31, 2023
For #39661.
For #41176.
For #51593.
For #52397.
For #57192.
For #58645.
For #58650.
For #58671.
For #59338.
For #59750.
For #60353.

Change-Id: Ib731c9f2879beb541f44cb10e40c36a8677d3ad4
Reviewed-on: https://go-review.googlesource.com/c/go/+/499282
TryBot-Bypass: Robert Griesemer <gri@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generics Issue is related to generics NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. TypeInference Issue is related to generic type inference
Projects
None yet
Development

No branches or pull requests

2 participants