-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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: add Func.Signature method #65772
Comments
Change https://go.dev/cl/565035 mentions this issue: |
Thanks for proposing this. "All Funcs have Signature type" is one of those non-obvious things that must be repeatedly recalled (why can't they have |
Change https://go.dev/cl/565375 mentions this issue: |
Would this API ever return nil or panic? Presumably not, but what if I called NewFunc with a nil signature, which seems to be allowed? |
Good question. I don't know why sig=nil was ever permitted, but it was, and it appears to be used (e.g. to construct special-purpose func symbols with no recv/tparams/params/results). I think we should interpret nil as the trivial signature, and make sig!=nil an invariant. |
I would agree with that. Perhaps nil was used to avoid allocations, in which case you could reuse a global empty signature, akin to other globals like the |
This change is the result of an audit of all type assertions and type switches whose operand is a types.Type. (These were enumerated with an analyzer tool.) If the operand is already the result of a call to Underlying or Unalias, there is nothing to do, but in other cases, explicit Unalias operations were added in order to preserve the existing behavior when go/types starts creating explicit Alias types. This change does not address any desired behavior changes required for the ideal handling of aliases; they will wait for a followup. In a number of places I have added comments matching "TODO.*alias". It may be prudent to split this change by top-level directory, both for ease of review, and of later bisection if needed. During the audit, there appeared to be a recurring need for the following operators: - (*types.Func).Signature (golang/go#65772); - Deref(Type): it's easy to forget to strip off the Alias constructor; - ReceiverName (CL 565075), for destructuring receiver types such as T and *T, in which up to two Aliases might be present. Updates golang/go#65294 Change-Id: I5180b9bae1c9191807026b8e0dc6f15ed4953b9a Reviewed-on: https://go-review.googlesource.com/c/tools/+/565035 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]> Auto-Submit: Alan Donovan <[email protected]>
Based on the discussion above, this proposal seems like a likely accept. The proposal is to add:
|
Based on #65772 (comment), should we specify in the doc comment what happens with a nil type? Or, @adonovan, are you proposing a change to |
It was never intended that nil was a valid argument to NewFunc, but it may have been exploited. We should change NewFunc to replace nil with a trivial non-nil Signature to preserve the invariant. |
No change in consensus, so accepted. 🎉 The proposal is to add:
|
Proposal Details
The expression
f.Type().(*types.Signature)
, where f is a*types.Func
, appears over a hundred times in x/tools. Each time I read it, I am momentarily compelled to prove a little theorem that the type assertion is sound.For brevity, convenience, and simplicity, I propose that we add this method to
go/types
:The text was updated successfully, but these errors were encountered: