Improve handling of Go symbols with type parameters #717
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For the curious: `bar(int (*) [5])` is a pointer to an array of length 5: typedef int IntArray[5]; int bar(IntArray* arr); For google#705.
Codecov Report
@@ Coverage Diff @@
## main #717 +/- ##
==========================================
+ Coverage 63.70% 64.21% +0.51%
==========================================
Files 41 41
Lines 6489 6495 +6
==========================================
+ Hits 4134 4171 +37
+ Misses 1913 1881 -32
- Partials 442 443 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. |
This was referenced Jul 28, 2022
Go symbols with type parameters are reported in Go pprof profiles as something like `main.Set[...]`. Today, multilinePrintableName mishandles this by replacing each with a newline, resulting in a very strange looking node. Instead, replace ... with a unicode ellipsis so that it isn't replaced with a newline just below. Note that the full symbol name (which is reported by tools like perf) is something like `[go.shape.string_0,go.shape.int_1]`. This case is still not handled very well, but we likely want to strip this out anyways in ShortenFunctionName or demangling as future work. Fixes google#705.
prattmic
force-pushed
the
generics_ellipsis
branch
from
5c43bb7
to
4a542c6
Compare
aalexand
approved these changes
Jul 29, 2022
gopherbot
pushed a commit
to golang/go
that referenced
this pull request
Aug 9, 2022
…bol names Updates github.com/google/pprof to bring in the commit from google/pprof#717 which fixes mangled symbol names for type parameters. Fixes #54105 Change-Id: I01af9f780aba3338b960a03b30906a23642e4448 Reviewed-on: https://go-review.googlesource.com/c/go/+/420234 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com> Reviewed-by: Than McIntosh <thanm@google.com>
jproberts
pushed a commit
to jproberts/go
that referenced
this pull request
Aug 10, 2022
…bol names Updates github.com/google/pprof to bring in the commit from google/pprof#717 which fixes mangled symbol names for type parameters. Fixes golang#54105 Change-Id: I01af9f780aba3338b960a03b30906a23642e4448 Reviewed-on: https://go-review.googlesource.com/c/go/+/420234 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com> Reviewed-by: Than McIntosh <thanm@google.com>
gopherbot
pushed a commit
to golang/go
that referenced
this pull request
Aug 19, 2022
…ngled type parameter symbol names Updates github.com/google/pprof to bring in the commit from google/pprof#717 which fixes mangled symbol names for type parameters. For #54105 Fixes #54420 Change-Id: I01af9f780aba3338b960a03b30906a23642e4448 Reviewed-on: https://go-review.googlesource.com/c/go/+/420234 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com> Reviewed-by: Than McIntosh <thanm@google.com> (cherry picked from commit cd9cd92) Reviewed-on: https://go-review.googlesource.com/c/go/+/423356 Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
bradfitz
pushed a commit
to tailscale/go
that referenced
this pull request
Sep 8, 2022
…ngled type parameter symbol names Updates github.com/google/pprof to bring in the commit from google/pprof#717 which fixes mangled symbol names for type parameters. For golang#54105 Fixes golang#54420 Change-Id: I01af9f780aba3338b960a03b30906a23642e4448 Reviewed-on: https://go-review.googlesource.com/c/go/+/420234 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com> Reviewed-by: Than McIntosh <thanm@google.com> (cherry picked from commit cd9cd92) Reviewed-on: https://go-review.googlesource.com/c/go/+/423356 Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes two issues:
main.(*Foo[...]).Method->main..Method..with newlines, converting[...]->[\n\n\n].See individual commit messages for more details.
Fixes #705