Skip to content

Go: Only output origin constraints for methods #22399

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

Merged
jketema merged 2 commits into from
Sep 2, 2026
+109 −24
Merged

Go: Only output origin constraints for methods #22399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
17afaf2
Go: Add test that shows that constraints might not be uniquely determ…
jketema Aug 20, 2026
6a9e2d5
Go: Only output origin constraints for methods
jketema Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 90 additions & 24 deletions go/extractor/extractor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,16 @@ type typeParamParentEntry struct {
isFromReceiver bool
}

// typeParamMutex protects typeParamParent.
var typeParamParentMutex sync.RWMutex

var typeParamParent map[*types.TypeParam]typeParamParentEntry = make(map[*types.TypeParam]typeParamParentEntry)

// typeParamMutex protects typeParamOrigin.
var typeParamOriginMutex sync.RWMutex

var typeParamOrigin map[*types.TypeParam]*types.TypeParam = make(map[*types.TypeParam]*types.TypeParam)

@jketema jketema Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. That also applies to the pre-existing typeParamParent

@jketema jketema Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@owen-mc owen-mc Aug 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@jketema jketema Aug 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


func init() {
// this sets the number of threads that the Go runtime will spawn; this is separate
// from the number of goroutines that the program spawns, which are scheduled into
Expand Down Expand Up @@ -1658,29 +1666,7 @@ func extractType(tw *trap.Writer, tp types.Type) trap.Label {
for i := 0; i < origintp.NumMethods(); i++ {
meth := origintp.Method(i).Origin()
extractMethod(tw, meth)

// Consider a generic struct and a generic method:
//
// type S[P any] struct{}
// func (*S[P]) m[Q any](x Q) {}
//
// If we have a variable 's' of type 'S[int]' and the expression
// 's.m[string]("")', then the type of the selector expression 's.m'
// is ' func(Q)'. The method 'm' here is an instantiation of the
// declaration, which has its own type with type parameter 'Q'.
// As we do not extract method instantiations, 'populateTypeParamParents'
// does not automatically get called for the type parameter 'Q'
// from the instantiation of 'm'. To compensate, we add the type
// parameters here.
//
// As a parent we use the origin method. This suffices, as the name
// and index of the type parameter in the instantiation will be
// identical to those of the uninstantiated method, and as only
// these two properties will be extracted for a type parameter.
if tp.Method(i) != meth {
signature := tp.Method(i).Type().(*types.Signature)
populateTypeParamParents(signature.TypeParams(), meth, false)
}
populateTypeParamParentsAndOrigins(tp.Method(i), meth)
}

underlyingInterface, underlyingIsInterface := underlying.(*types.Interface)
Expand All @@ -1704,7 +1690,8 @@ func extractType(tw *trap.Writer, tp types.Type) trap.Label {
case *types.TypeParam:
kind = dbscheme.TypeParamType.Index()
parentlbl, isReceiverChild := getTypeParamParentLabel(tw, tp)
constraintLabel := extractType(tw, tp.Constraint())
constraint := getTypeParamOrigin(tp).Constraint()
constraintLabel := extractType(tw, constraint)
dbscheme.TypeParamTable.Emit(tw, lbl, tp.Obj().Name(), constraintLabel, parentlbl, tp.Index(), isReceiverChild)
case *types.Union:
kind = dbscheme.TypeSetLiteral.Index()
Expand Down Expand Up @@ -2062,7 +2049,10 @@ func getObjectBeingUsed(tw *trap.Writer, ident *ast.Ident) types.Object {
}

func getTypeParamParentLabel(tw *trap.Writer, tp *types.TypeParam) (trap.Label, bool) {
typeParamParentMutex.RLock()
entry, exists := typeParamParent[tp]
typeParamParentMutex.RUnlock()

if !exists {
log.Fatalf("Parent of type parameter does not exist: %s %s", tp.String(), tp.Constraint().String())
}
Expand All @@ -2074,6 +2064,9 @@ func getTypeParamParentLabel(tw *trap.Writer, tp *types.TypeParam) (trap.Label,
}

func setTypeParamParent(tp *types.TypeParam, parent types.Object, isFromReceiver bool) {
typeParamParentMutex.Lock()
defer typeParamParentMutex.Unlock()

entry, exists := typeParamParent[tp]
newEntry := typeParamParentEntry{parent, isFromReceiver}
if !exists {
Expand Down Expand Up @@ -2121,3 +2114,76 @@ func checkObjectNotSpecialized(obj types.Object) {
}
}
}

// getTypeParamOrigin returns the origin type parameter for a type parameter
// from an instantiated method.
func getTypeParamOrigin(tp *types.TypeParam) *types.TypeParam {
typeParamOriginMutex.RLock()
origin, exists := typeParamOrigin[tp]
typeParamOriginMutex.RUnlock()

if exists {
return origin
} else {
return tp
}
}

// populateTypeParamParentsAndOrigins records for each type parameter of a method
// the origin parent and type parameter.
//
// Consider a generic struct and a generic method:
//
// type S[P any] struct{}
// func (*S[P]) m[Q ~P](x Q) {}
//
// If we have a variable 's' of type 'S[int]' and the expression 's.m[int](42)',
// then the type of the selector expression 's.m' is 'func[Q ~int](Q)'. The
// method 'm' here is an instantiation of the declaration, which has its own
// type with type parameter 'Q' with constraint 'interface { ~int }'. As we
// do not extract method instantiations, but only their origins, we want to
// match this behavior for the constraints of instantiated type parameter, and
// record their origin. Moreover, not extracting instantiations also means that
// 'populateTypeParamParents' does not automatically get called on their type
// parameters. To compensate, we add the type params here by calling
// `setTypeParamParent`.
//
// As the parent of a type parameter use the origin method. This suffices, as
// the name and index of the type parameter in the instantiation will be
// identical to those of the uninstantiated method, and as only a constraint and
// these two properties will be extracted for a type parameter.
func populateTypeParamParentsAndOrigins(meth *types.Func, originmeth *types.Func) {
if meth == originmeth {
return
}

typeparams := meth.Type().(*types.Signature).TypeParams()
populateTypeParamParents(typeparams, originmeth, false)

origintypeparams := originmeth.Type().(*types.Signature).TypeParams()
populateTypeParamOrigins(meth, typeparams, origintypeparams)
}

func populateTypeParamOrigins(meth *types.Func, typeparams *types.TypeParamList, origintypeparams *types.TypeParamList) {
if typeparams.Len() != origintypeparams.Len() {
log.Fatalf("Method instantiation %s has %d type parameters, origin has %d",
meth, typeparams.Len(), origintypeparams.Len())
}

for j := 0; j < typeparams.Len(); j++ {
setTypeParamOrigin(typeparams.At(j), origintypeparams.At(j))
}
}

func setTypeParamOrigin(typeparam *types.TypeParam, origintypeparam *types.TypeParam) {
typeParamOriginMutex.Lock()
defer typeParamOriginMutex.Unlock()

entry, exists := typeParamOrigin[typeparam]
if !exists {
typeParamOrigin[typeparam] = origintypeparam
} else if entry != origintypeparam {
log.Fatalf("Origin of type parameter '%s %s' being set to a different value: '%s' vs '%s'",
typeparam.String(), typeparam.Constraint().String(), entry.String(), origintypeparam.String())
}
}
5 changes: 5 additions & 0 deletions go/ql/test/library-tests/semmle/go/Function/TypeParamType.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ numberOfTypeParameters
| genericMethods.go:5:33:5:46 | GenericMethod1 | 1 |
| genericMethods.go:7:6:7:28 | StructForGenericMethod2 | 1 |
| genericMethods.go:9:37:9:50 | GenericMethod2 | 2 |
| genericMethods.go:21:6:21:29 | StructWithDependentBound | 1 |
| genericMethods.go:23:38:23:68 | GenericMethodWithDependentBound | 2 |
#select
| codeql-go-tests/function.EdgeConstraint | 0 | | Node | interface { } |
| codeql-go-tests/function.Element | 0 | | S | interface { } |
Expand Down Expand Up @@ -51,6 +53,9 @@ numberOfTypeParameters
| codeql-go-tests/function.StructForGenericMethod2 | 0 | | P2 | interface { } |
| codeql-go-tests/function.StructForGenericMethod2.GenericMethod2 | 0 | | P4 | interface { } |
| codeql-go-tests/function.StructForGenericMethod2.GenericMethod2 | 0 | from receiver | P3 | interface { } |
| codeql-go-tests/function.StructWithDependentBound | 0 | | P5 | interface { } |
| codeql-go-tests/function.StructWithDependentBound.GenericMethodWithDependentBound | 0 | | P7 | interface { ~[]P6 } |
| codeql-go-tests/function.StructWithDependentBound.GenericMethodWithDependentBound | 0 | from receiver | P6 | interface { } |
| codeql-go-tests/function.multipleAnonymousTypeParamsFunc | 0 | | _ | interface { } |
| codeql-go-tests/function.multipleAnonymousTypeParamsFunc | 1 | | _ | interface { string } |
| codeql-go-tests/function.multipleAnonymousTypeParamsFunc | 2 | | _ | interface { } |
Expand Down
9 changes: 9 additions & 0 deletions go/ql/test/library-tests/semmle/go/Function/genericMethods.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,12 @@ func generic_methods(s1 StructForGenericMethod1, s2 StructForGenericMethod2[int]
s1.GenericMethod1("hello")
s2.GenericMethod2(42)
}

type StructWithDependentBound[P5 any] struct{}

func (*StructWithDependentBound[P6]) GenericMethodWithDependentBound[P7 ~[]P6](x P7) {}

func genericMethodDependentBounds(t1 StructWithDependentBound[int], t2 StructWithDependentBound[string]) {
t1.GenericMethodWithDependentBound([]int{})
t2.GenericMethodWithDependentBound([]string{})
}
3 changes: 3 additions & 0 deletions go/ql/test/library-tests/semmle/go/Function/getParameter.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
| genericMethods.go:9:37:9:50 | GenericMethod2 | 0 | genericMethods.go:9:60:9:60 | x |
| genericMethods.go:11:6:11:20 | generic_methods | 0 | genericMethods.go:11:22:11:23 | s1 |
| genericMethods.go:11:6:11:20 | generic_methods | 1 | genericMethods.go:11:50:11:51 | s2 |
| genericMethods.go:23:38:23:68 | GenericMethodWithDependentBound | 0 | genericMethods.go:23:80:23:80 | x |
| genericMethods.go:25:6:25:33 | genericMethodDependentBounds | 0 | genericMethods.go:25:35:25:36 | t1 |
| genericMethods.go:25:6:25:33 | genericMethodDependentBounds | 1 | genericMethods.go:25:69:25:70 | t2 |
| main.go:7:6:7:7 | f1 | 0 | main.go:7:9:7:9 | x |
| main.go:9:12:9:13 | f2 | 0 | main.go:9:15:9:15 | x |
| main.go:9:12:9:13 | f2 | 1 | main.go:9:18:9:18 | y |
Expand Down
2 changes: 2 additions & 0 deletions go/ql/test/library-tests/semmle/go/Function/getTypeParameter.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,5 @@
| genericMethods.go:5:1:5:63 | function declaration | MethodDecl | 0 | genericMethods.go:5:48:5:53 | type parameter declaration | 0 | genericMethods.go:5:48:5:49 | P1 | genericMethods.go:5:51:5:53 | any | interface { } |
| 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 } |

@owen-mc owen-mc Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Loading