Skip to content

go/parser: uses of function type params are not resolved in the signature #45221

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
Closed
findleyr opened this issue Mar 24, 2021 · 5 comments
Closed

go/parser: uses of function type params are not resolved in the signature #45221

findleyr opened this issue Mar 24, 2021 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.18

Comments

@findleyr
Copy link
Member

findleyr commented Mar 24, 2021 •
edited
Loading

go/parser object resolution correctly resolves type parameters within type declarations, but not within function declarations. For example, none of the uses of T in the below snippet are resolved (correction: the T in var t T is resolved, but not the Ts in the signature):

func Add[T Addable](l, r T) T {
        var t T
        return l + r + t
}

This is minor, but we should probably fix it. Will do as part of #45104.

CC @griesemer
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/304456 mentions this issue: go/parser: move type params in scope for the function signature

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/304453 mentions this issue: go/parser: add resolution tests for type params

@findleyr findleyr changed the title go/parser: identifiers referring to function type param are not resolved go/parser: uses of function type params are not resolved in the signature Mar 25, 2021
@cagedmantis cagedmantis added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 29, 2021
@cagedmantis cagedmantis added this to the Backlog milestone Mar 29, 2021
@cagedmantis
Copy link
Contributor

/cc @@griesemer

gopherbot pushed a commit that referenced this issue Mar 31, 2021
@findleyr
go/parser: add resolution tests for type params
152ca79 
For #45104
For #45221

Change-Id: I8966555f4e8844d5b6766d00d48f7a81868ccf40
Reviewed-on: https://go-review.googlesource.com/c/go/+/304453
Trust: Robert Findley <[email protected]>
Trust: Robert Griesemer <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
@findleyr findleyr self-assigned this Apr 3, 2021
gopherbot pushed a commit that referenced this issue Apr 16, 2021
@findleyr
go/parser: move type params in scope for the function signature
9e8a312 
Type parameter resolution is a bit tricky: type parameters are in the
function scope, but unlike ordinary parameters may reference eachother.
When resolving the function scope, we must be careful about the order in
which objects are resolved and declared.

Using ordering allows us to avoid passing around temporary scopes for
field declarations.

Add a bunch of tests for this behavior, and skip "_" in resolution tests
as it just adds noise.

For #45221

Change-Id: Id080cddce3fd76396bf86ba5aba856aedf64a458
Reviewed-on: https://go-review.googlesource.com/c/go/+/304456
Trust: Robert Findley <[email protected]>
Trust: Robert Griesemer <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
@findleyr findleyr modified the milestones: Backlog, Go1.18 Apr 28, 2021
@ianlancetaylor
Copy link
Contributor

@findleyr This is in the 1.18 milestone. Did it get fixed? Or should it move to 1.19? Thanks.

@findleyr findleyr mentioned this issue Feb 1, 2022
Open
@findleyr
Copy link
Member Author

findleyr commented Feb 2, 2022

Yes, this is done now. There is one remaining issue, which we can track with #50956.

@findleyr findleyr closed this as completed Feb 2, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
4 participants
@cagedmantis @ianlancetaylor @gopherbot @findleyr