-
Notifications
You must be signed in to change notification settings - Fork 18k
reflect: use Value.Len instead of conversion to slice header #48346
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
Conversation
This PR (HEAD: d1ac000) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/349469 to see it. Tip: You can toggle comments from me using the |
Message from Go Bot: Patch Set 1: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be During May-July and Nov-Jan the Go project is in a code freeze, during which Please don’t reply on this GitHub thread. Visit golang.org/cl/349469. |
Upstream uses a conversion to unsafeheader.Slice, which isn't supported by GopherJS. Using Value.Len() is preferrable, since we already override it to provide compatibility. If/when golang/go#48346 gets into the stable release, this patch will no longer be necessary.
Message from Keith Randall: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/349469. |
Message from Alexander Zhirov: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/349469. |
Message from Keith Randall: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/349469. |
This change is functionally equivalent, but reduces reliance on unsafe features. This would allow GopherJS to avoid an additional patch to the standard library we'd have to maintain in order to remain compatible with Go 1.17+.
d1ac000
to
94ebb39
Compare
This PR (HEAD: 94ebb39) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/349469 to see it. Tip: You can toggle comments from me using the |
Message from Alexander Zhirov: Patch Set 2: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/349469. |
Message from Keith Randall: Patch Set 2: Run-TryBot+1 Code-Review+2 Trust+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/349469. |
Message from Go Bot: Patch Set 2: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/349469. |
Message from Go Bot: Patch Set 2: TryBot-Result+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/349469. |
This change is functionally equivalent, but reduces reliance on unsafe features. This would allow GopherJS to avoid an additional patch to the standard library we'd have to maintain in order to remain compatible with Go 1.17+. Change-Id: I4f113db0c572ec0b81ebfecf5a137145f6c8c41d GitHub-Last-Rev: 94ebb39 GitHub-Pull-Request: #48346 Reviewed-on: https://go-review.googlesource.com/c/go/+/349469 Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> Trust: Keith Randall <[email protected]> Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Go Bot <[email protected]>
This PR is being closed because golang.org/cl/349469 has been merged. |
This change is functionally equivalent, but reduces reliance on unsafe
features. This would allow GopherJS to avoid an additional patch to the
standard library we'd have to maintain in order to remain compatible
with Go 1.17+.