-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
cmd/compile: clarify Type.IsPtr and Isptr #15028
Comments
Seems like a good idea to me, regarding the front-end. I let @randall77 comment on the SSA change. Sometimes IsPtr may need to include IsUnsafePtr, and sometimes not. Depending on how many use cases of each kind you may want to make IsPtr include unsafe.Pointer, or not. It'll be obvious enough once the rewrite is done. |
Aside: I'm wondering if IsPtrShaped should include the extra checks for single-element structs and arrays that isdirectiface has. It's not obvious to me that there are use cases for wanting to identify channel, map, and func types because they fit into a single pointer value, but not single element struct/arrays that do too. I haven't looked much into that yet though. |
I guess I'm in good company. :) The methods could use some docs, a la the discussion above. Are you also planning to send a CL switching the front-end to use IsPtr?
It's complicated. See the comment at |
Previously, t.IsPtr() reported whether t was represented with a pointer, but some of its callers expected it to report whether t is an actual Go pointer. Resolve this by renaming t.IsPtr to t.IsPtrShaped and adding a new t.IsPtr method to report Go pointer types. Updated a couple callers in gc/ssa.go to use IsPtr instead of IsPtrShaped. Passes toolstash -cmp. Updates #15028. Change-Id: I0a8154b5822ad8a6ad296419126ad01a3d2a5dc5 Reviewed-on: https://go-review.googlesource.com/21232 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Yeah. I was going to try to get rid of all the Isslice, Isfixedarray, Isptr stuff in favor of the Type methods that were added for SSA. |
Fabulous. We are in danger of stepping on each others' toes with large scale automated rewrites, but I think/hope that mine should be done pretty soon. Type is actually getting closer to being extractable into its own package. Hard to believe. |
func (t *Type) IsPtr() bool
was introduced for the SSA backend. It reports (roughly) whether type is pointer-shaped, so it returns true for e.g. maps and channels. The "roughly" qualifier is because there are some things that are pointer-shaped (struct { i *int }
) for which it returns false.var Isptr [NTYPE]bool
is used extensively in the frontend. It is set to true for only TPTR32 and TPTR64.This doesn't matter directly to my type encapsulation efforts, but it seems confusing/dangerous. Here's a proposal. I'd like feedback before I implement it, because the CL will be large, albeit mostly automated:
IsPtr
method toIsPtrShaped
and adjust ssa.Type appropriately. Use docs to explain the caveat noted above.Isptr
var in favor of anIsPtr
method on Type.IsUnsafePtr
method on Type to highlight thatIsPtr
does not include TUNSAFEPTR.IsPtr
to ensure that the right pointer width is in use everywhere.@randall77 @mdempsky @bradfitz
The text was updated successfully, but these errors were encountered: