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

Jump to bottom

go/types, types2: typeparams: sanitize Scopes, etc #46151

Closed
findleyr opened this issue May 13, 2021 · 3 comments
Closed

go/types, types2: typeparams: sanitize Scopes, etc #46151

findleyr opened this issue May 13, 2021 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.18

Comments

@findleyr
Copy link
Contributor

findleyr commented May 13, 2021 •
edited

Reminder issue: unexpanded *instance types, which are meant to exist only within the type checking pass, are currently leaking in a number of places, most notably within Scopes.

It might be possible to avoid sanitization entirely, by packing the *instance data into *Named, and nilling out upon expansion, similar to what is done in https://golang.org/cl/318849.

CC @griesemer
@findleyr findleyr added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 13, 2021
@findleyr findleyr added this to the Go1.18 milestone May 13, 2021
@findleyr findleyr self-assigned this May 13, 2021
@findleyr findleyr changed the title go/types, types2: typeparams: sanitize Scopes, Named.Orig, etc go/types, types2: typeparams: sanitize Scopes, etc May 13, 2021
@findleyr
Copy link
Contributor Author

findleyr commented Jun 22, 2021 •
edited

This would be addressed by https://golang.org/cl/328153, exposing the Instance type and eliminating the need for sanitization.

@gopherbot
Copy link

Change https://golang.org/cl/335929 mentions this issue: [dev.typeparams] go/types: merge instance and Named to eliminate sanitization

@gopherbot
Copy link

Change https://golang.org/cl/336252 mentions this issue: [dev.typeparams] cmd/compile/internal/types2: merge instance and Named to eliminate sanitization

gopherbot pushed a commit that referenced this issue Jul 22, 2021
@findleyr
[dev.typeparams] go/types: merge instance and Named to eliminate sani…
61f69d2 
…tization

Storing temporary syntactic information using an *instance type forces
us to be careful not to leak references to *instance in the checker
output. This is complex and error prone, as types are written in many
places during type checking.

Instead, temporarily pin the necessary syntactic information directly to
the Named type during the type checking pass. This allows us to avoid
having to sanitize references.

This includes a couple of small, unrelated changes that were made in the
process of debugging:
 - eliminate the expandf indirection: it is no longer necessary
 - include type parameters when printing objects

For #46151

Change-Id: I767e35b289f2fea512a168997af0f861cd242175
Reviewed-on: https://go-review.googlesource.com/c/go/+/335929
Trust: Robert Findley <rfindley@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
@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 NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
2 participants
@gopherbot @findleyr