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

reflect: TypeOf(unsafe.Pointer(nil)).PkgPath() returns "", not "unsafe" #44830

Closed
mdempsky opened this issue Mar 6, 2021 · 9 comments
Closed
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@mdempsky
Copy link
Member

mdempsky commented Mar 6, 2021

The documentation for reflect.Type.PkgPath says:

    // PkgPath returns a defined type's package path, that is, the import path
    // that uniquely identifies the package, such as "encoding/base64".
    // If the type was predeclared (string, error) or not defined (*T, struct{},
    // []int, or A where A is an alias for a non-defined type), the package path
    // will be the empty string.

unsafe.Pointer is not listed as a predeclared type: https://golang.org/ref/spec#Predeclared_identifiers (Though go/types lists it as one: https://golang.org/pkg/go/types/#BasicKind)

So it seems to reason that PkgPath should return "unsafe", but it instead returns "": https://play.golang.org/p/ZnUOgHjwz58

/cc @griesemer @ianlancetaylor

@mdempsky mdempsky added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 6, 2021
@mdempsky mdempsky added this to the Go1.17 milestone Mar 6, 2021
@odeke-em
Copy link
Member

odeke-em commented Mar 7, 2021

@mdempsky, thank you for catching this discrepancy, nice work! If I may ask, perhaps should we repurpose this as a proposal to include unsafe.Pointer into builtin but also to update the spec? I think this was just an oversight from commit 20850fc almost 12 years ago in which we perhaps forgot to update the spec. @robpike @rsc @griesemer @ianlancetaylor is my assumption correct? Thank you.

@mdempsky
Copy link
Member Author

mdempsky commented Mar 7, 2021

unsafe.Pointer is mentioned in the spec. It's also declared in package unsafe, not the universe, so it's properly not listed as a predeclared type/identifier. I don't see any reason to change the spec here.

@ianlancetaylor
Copy link
Contributor

At first glance this seems like a compiler bug to me. It's probably treating unsafe.Pointer as a predeclared type rather than as a type defined in the "unsafe" package, so it's probably generating an uncommonType struct with a zero pkgPath field. Or something like that.

@mdempsky
Copy link
Member Author

mdempsky commented Mar 8, 2021

I can believe it's actually a compiler bug.

FWIW, it looks like with gccgo 11, both PkgPath and also Name return "".

@ianlancetaylor
Copy link
Contributor

For gccgo it's a libgo bug, because libgo holds the unique type descriptor for unsafe.Pointer (https://go.googlesource.com/gofrontend/+/refs/heads/master/libgo/runtime/go-unsafe-pointer.c). But as far as I know the gc compiler doesn't work that way.

@mdempsky
Copy link
Member Author

mdempsky commented Mar 8, 2021

Sure. I'm just pointing out a similar issue also affects gccgo.

@go101
Copy link

go101 commented Mar 9, 2021

It looks Go spec views unsafe things as builtin things in several places.

  • ... or the result value of some built-in functions such as unsafe.Sizeof applied to any value, ...
  • The following built-in functions are not permitted in statement context: ... unsafe.Alignof unsafe.Offsetof unsafe.Sizeof

@gopherbot
Copy link

Change https://golang.org/cl/313349 mentions this issue: cmd/compile: fix wrong package path for unsafe.Pointer

@gopherbot
Copy link

Change https://golang.org/cl/313890 mentions this issue: all: clarify that unsafe.Pointer is not a predeclared type

nevkontakte added a commit to nevkontakte/gopherjs that referenced this issue Sep 17, 2021
@golang golang locked and limited conversation to collaborators Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

5 participants