Uh oh!
There was an error while loading. Please reload this page.
Go: Only output origin constraints for methods - #22399
Conversation
bb92ed7 to
2178247CompareThere was a problem hiding this comment.
Pull request overview
Ensures instantiated generic Go methods emit their origin type-parameter constraints.
Changes:
- Tracks origin type parameters for instantiated methods.
- Adds dependent-bound generic method coverage.
- Updates expected query results.
Show a summary per file
| File | Description |
|---|---|
go/extractor/extractor.go | Maps instantiated method parameters to origin constraints. |
genericMethods.go | Adds dependent-bound test cases. |
TypeParamType.expected | Updates type-parameter results. |
getTypeParameter.expected | Updates declaration results. |
getParameter.expected | Updates parameter results. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Balanced
| var typeParamParent map[*types.TypeParam]typeParamParentEntry = make(map[*types.TypeParam]typeParamParentEntry) | ||
| var typeParamOrigin map[*types.TypeParam]*types.TypeParam = make(map[*types.TypeParam]*types.TypeParam) |
There was a problem hiding this comment.
Interesting. That also applies to the pre-existing typeParamParent
There was a problem hiding this comment.
I convinced myself that we wouldn't have synchronization issues in the old behaviour. Each thread works independently. Even if they refer to the same type they make their own classes to represent that type. And we always populate an entry in the map before we try to read it. I don't understand the new behaviour well enough to tell if that's still true. I guess you might want add another test file which references GenericMethodWithDependentBound to double check that that works.
There was a problem hiding this comment.
I don't believe that was a sufficient condition even before the introduction of generic methods. My understanding is that concurrent writes and concurrent read-writes are simply not safe, even if the elements read/written are different.
Uh oh!
There was an error while loading. Please reload this page.
b58033f to
293f687CompareAlso mutex protect the two global maps. Go `map`s are not are not thread safe in the case of write/writes and read/writes.
293f687 to
6a9e2d5Compare
owen-mc
left a comment
There was a problem hiding this comment.
This seems to fix a real problem.
| | genericMethods.go:7:6:7:45 | type declaration specifier | TypeSpec | 0 | genericMethods.go:7:30:7:35 | type parameter declaration | 0 | genericMethods.go:7:30:7:31 | P2 | genericMethods.go:7:33:7:35 | any | interface { } | | ||
| | genericMethods.go:9:1:9:67 | function declaration | MethodDecl | 0 | genericMethods.go:9:52:9:57 | type parameter declaration | 0 | genericMethods.go:9:52:9:53 | P4 | genericMethods.go:9:55:9:57 | any | interface { } | | ||
| | genericMethods.go:21:6:21:46 | type declaration specifier | TypeSpec | 0 | genericMethods.go:21:31:21:36 | type parameter declaration | 0 | genericMethods.go:21:31:21:32 | P5 | genericMethods.go:21:34:21:36 | any | interface { } | | ||
| | genericMethods.go:23:1:23:87 | function declaration | MethodDecl | 0 | genericMethods.go:23:70:23:77 | type parameter declaration | 0 | genericMethods.go:23:70:23:71 | P7 | genericMethods.go:23:73:23:77 | type set literal | interface { ~[]P6 } | |
There was a problem hiding this comment.
Note that there should be another line here for P6 on line 23. But I believe this is an existing bug that we don't properly extract everything about type parameters defined on the receiver, so it shouldn't affect this PR. I can't remember - maybe we don't extract the object, or the declaration? I tried to fix it years ago but it wasn't easy. Maybe the new map you've introduced will make it easy, as it allows you link a type parameter to the place where it was declared.
Uh oh!
There was an error while loading. Please reload this page.
Addresses #22042 (comment) by only outputting the origins of constraints on generic method and not instantiated constraints. This matches the behavior in other places where we have both instantiations and origins.