handle demangling OSX names with underscore prefix#924
Conversation
| // OSX has all the symbols prefixed with extra '_' so lets try | ||
| // once more without it | ||
| if strings.HasPrefix(fn.SystemName, "_") { | ||
| if demangled := demangle.Filter(fn.SystemName[1:], o...); demangled != fn.SystemName { |
There was a problem hiding this comment.
Could we just pass strings.StripPrefix(fn.SystemName, "_"), ... in the demangle.Filter call above?
There was a problem hiding this comment.
Above? So just cut all the time no matter OS and everything? Then it won't work, since _ is usually used for mangled names for regular OS-es. And cutting it will simply break everything.
There was a problem hiding this comment.
Oh, got it.
Can we add a test that will exercise this code path?
Updates github ticket google#477
7dd4dbb to
85c67d0
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #924 +/- ##
==========================================
+ Coverage 66.86% 66.96% +0.09%
==========================================
Files 44 44
Lines 9824 9864 +40
==========================================
+ Hits 6569 6605 +36
- Misses 2794 2820 +26
+ Partials 461 439 -22 ☔ View full report in Codecov by Sentry. |
|
The failure looks like a flake. The failed test looks unrelated to my change. But do let me know if I am mistaken. And if there is anything I can do to help. |
Right, it's a known recent flaky error, I filed #925 to track it. I restarted the failed test. |
|
Thanks. Test passed now. So what happens next ? |
|
merged |
This is a follow-up to google#924. In the comparison we should compare to the string with stripped underscore, otherwise the comparison will never be true and we strip the underscore in cases we shouldn't. See the added test case. Also, while we are here, add copying demangling options that we also missed. Hopefully we can get ianlancetaylor/demangle#24 in and this won't be needed.
This is a follow-up to #924. In the comparison we should compare to the string with stripped underscore, otherwise the comparison will never be true and we strip the underscore in cases we shouldn't. See the added test case. Also, while we are here, add copying demangling options that we also missed. Hopefully we can get ianlancetaylor/demangle#24 in and this won't be needed.
3 participants
See discussion at #477 (comment)