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.

Sign up for GitHub

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

x/tools/gopls: type constraint discrepancy between go compiler and gopls #61609

Open
marekscholle opened this issue Jul 27, 2023 · 8 comments
Open

x/tools/gopls: type constraint discrepancy between go compiler and gopls #61609

marekscholle opened this issue Jul 27, 2023 · 8 comments
Assignees
@findleyr
Labels
gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.17.0

Comments

@marekscholle
Copy link

Working on Go project In VSCode, I noticed gopls reports a problem which is ok for the compiler.

I extracted a minimal example as follows:

Suppose we have a project with packages foo (file foo.go) and bar (file bar.go).

The package foo exports type constraint A:

// foo.go
package foo

type A comparable

The package bar consumes the constraint A as

// bar.go
package bar

// import "foo"

func F[T foo.A](a T, b T) bool {
	return a == b
}

Since the constraint A "inherits" from comparable, the expression a == b is well defined: T is constrained by A which is comparable, hence == is available for values of type T. The compiler is happy with the code.

But VSCode reports a == b as the problem:

invalid operation: a == b (incomparable types in type set)

In editor,
image

Two observations:

  • if the constraint type A comparable is defined in the package bar, it's ok for gopls – it's an cross-package problem
  • if the constraint is defined as type A = comparable ie. as typedef (not "newtype") in package foo, it's also ok
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Jul 27, 2023
@hyangah hyangah changed the title Type constraint discrepancy between go compiler and gopls x/tools/gopls: type constraint discrepancy between go compiler and gopls Jul 27, 2023
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jul 27, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jul 27, 2023
@hyangah
Copy link
Contributor

hyangah commented Jul 27, 2023

I guess this is an issue in go/types.
The problem is reproducible with gopls built with go1.21rc3 and gopls v0.13.0-pre.2.

cc @findleyr @griesemer @adonovan

@hyangah hyangah added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 27, 2023
@findleyr
Copy link
Contributor

Based on the description, I suspect an issue with export/import.

I will take a look with urgency. It would be good to fix this for v0.13.0.

@findleyr
Copy link
Contributor

Based on the description, I suspect an issue with export/import.

Confirmed. If I turn off export data, the problem goes away.

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.13.1 Jul 27, 2023
@findleyr
Copy link
Contributor

So, the problem is that named type forwarding breaks the relationship between the builtin comparable and the underlying of A. That is to say: the resulting type A has an underlying that is the empty interface, with the hidden comparable bit set.

Note that type A interface{comparable} is effectively the same type, but doesn't have this problem, because the comparable bit is encoded as the explicitly embedded comparable type.

Fixing this is a bit tricky, because there is no way to access or set the comparable bit explicitly in the go/types API. The only thing we can do is recognize this specific case by checking for pointer equality between A.Underlying() and types.Universe.Lookup("comparable").Underlying(). Feels a bit dirty, but should work.

This will require careful review, so won't make the v0.13.0 release. Moving to v0.13.1.

@findleyr findleyr added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 27, 2023
@findleyr
Copy link
Contributor

Also meant to say: thanks for the careful issue report! It made tracking this down much easier.

@marekscholle
Copy link
Author

Thanks for providing the workaround type A interface{comparable} – it worked for me :)

1 similar comment
@marekscholle
Copy link
Author

Thanks for providing the workaround type A interface{comparable} – it worked for me :)

@findleyr findleyr self-assigned this Jul 27, 2023
@findleyr findleyr modified the milestones: gopls/v0.13.1, gopls/v0.13.2 Aug 1, 2023
@findleyr
Copy link
Contributor

Since this is relatively unlikely, and has a simple workaround, punting to gopls@v0.14.0.

@findleyr findleyr modified the milestones: gopls/v0.13.2, gopls/v0.14.0 Aug 11, 2023
@findleyr findleyr modified the milestones: gopls/v0.14.0, gopls/v0.15.0 Oct 9, 2023
@findleyr findleyr modified the milestones: gopls/v0.15.0, gopls/v0.16.0 Jan 17, 2024
@findleyr findleyr modified the milestones: gopls/v0.16.0, gopls/v0.17.0 Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@findleyr findleyr

Labels
gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.17.0
Development

No branches or pull requests
4 participants
@hyangah @gopherbot @marekscholle @findleyr