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

go/types, types2: writeType does not invoke Qualifier when printing an unsafe.Pointer #44515

Closed
cixel opened this issue Feb 22, 2021 · 4 comments
Closed
Assignees
Milestone

Comments

@cixel
Copy link

@cixel cixel commented Feb 22, 2021

Because unsafe.Pointer is a *types.Basic, the type stringifier uses a special case to print the package name:

case *Basic:
	if t.kind == UnsafePointer {
		buf.WriteString("unsafe.")
	}

As written, this does not allow a given Qualifier function to run and control how the package name is printed, which means that anything which wants to print something other than "unsafe" must do so by also special casing unsafe.Pointer.

Is there any reason not to use the Qualifier function in writeType when printing unsafe.Pointer? This would be a very quick change, but I wanted to make sure the current functionality isn't deliberate before opening a CL.

@findleyr
Copy link
Contributor

@findleyr findleyr commented Feb 23, 2021

This seems reasonable to me. Given that this logic predates https://golang.org/cl/11692 adding the Qualifier function, perhaps this was just an oversight.

CC @griesemer

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 23, 2021

Change https://golang.org/cl/295271 mentions this issue: cmd/compile/internal/types2: use regular type printing for unsafe.Pointer

Loading

@griesemer
Copy link
Contributor

@griesemer griesemer commented Feb 23, 2021

Thanks for pointing this out; I think @findleyr is correct, this looks like an oversight. Fix is out.

Loading

@griesemer griesemer self-assigned this Feb 23, 2021
@griesemer griesemer added this to the Go1.17 milestone Feb 23, 2021
@griesemer griesemer changed the title go/types: writeType does not invoke Qualifier when printing an unsafe.Pointer go/types, types2: writeType does not invoke Qualifier when printing an unsafe.Pointer Feb 23, 2021
@gopherbot gopherbot closed this in a2e150c Feb 23, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Feb 25, 2021

Change https://golang.org/cl/296289 mentions this issue: cmd/guru: update golden file (fix test failure)

Loading

gopherbot pushed a commit to golang/tools that referenced this issue Feb 25, 2021
The go/types fix for golang/go#44515 changed the type string for unsafe.Pointer
from a fixed "unsafe.Pointer" to a customizable package path followed
by "Pointer" (the customization was in place for any other object already).
The package path customization is done through a user-provider types.Qualifier
function. If none is provided, the ordinary package path is used ("unsafe").

Guru provides a package-relative qualifier which leaves away the package
path if an object is local to a package. As a result, unsafe.Pointer is
printed as "Pointer" which breaks an existing test.

Provide no qualifier function when printing a type from package unsafe
instead (there's only unsafe.Pointer). Since no qualifier was used in
the past, this Guru-specific change will also work when run using earlier
Go versions.

Fixes golang/go#44596.
Updates golang/go#44515.

Change-Id: I3c467e4ed09aa13deb50368fe98e42c723a6376b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/296289
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants