Add support for demangling C++ frames' symbols #31
Conversation
fitzgen
force-pushed
the
cpp_demangle
branch
from
283f0d3 to
9aea357
Compare
|
Woops, ok force pushed a fix for building without the |
src/symbolize/mod.rs
Outdated
| pub fn as_str(&self) -> Option<&'a str> { | ||
| self.demangled.as_ref().map(|s| s.as_str()) | ||
| /// Returns the raw symbol name as a `str` if the symbols is valid utf-8. | ||
| pub fn as_str(&self) -> Option<Cow<'a, str>> { |
There was a problem hiding this comment.
Could this stay as it was but leave a TODO to fix this on the next major version change?
|
Looks great to me, thanks! Perhaps though this could be off by default to start off with? Also, could you add just a smoke test or two that it works? |
fitzgen
force-pushed
the
cpp_demangle
branch
from
9aea357 to
a135194
Compare
|
Ran into some issues:
Because of those two things, I don't think we can proceed. I think we need to fix the |
|
@fitzgen it looks like there may be some |
|
Oh wait that's int he crate, carry on! |
|
It looks like the tests may also be failing on Linux because there's some inlined functions in libstd? |
|
Yeah I realized this wasn't ready and I need to rewrite parts of it to leverage |
|
I'll ping you again once I've done that. Thanks! |
fitzgen
force-pushed
the
cpp_demangle
branch
from
b9c54b3 to
cd3fda2
Compare
|
@alexcrichton OK, assuming CI passes, I think this is ready. |
build.rs
Outdated
| // > #[cfg(all(test, feature = "demangle_cpp"))] | ||
| // | ||
| // but it did not work... | ||
| #[cfg(feature = "demangle_cpp")] |
There was a problem hiding this comment.
This is unfortunate. Perhaps you know how we can only link the test C++ code when building for tests?
There was a problem hiding this comment.
Ah yeah unfortunately this isn't possible right now, to work around it we'll have to move it to a whole separate crate which is cargo test'd.
fitzgen
force-pushed
the
cpp_demangle
branch
3 times, most recently
from
73decea to
63e76b3
Compare
|
Ok, moved the C++ smoke test to a sub-crate. Hopefully this did not break CI... |
This adds a new optional (but on-by-default) dependency on the `cpp_demangle` crate. When this feature is enabled, if demangling a frame's symbol as a Rust symbol fails, then we attempt to demangle it as a C++ symbol.
fitzgen
force-pushed
the
cpp_demangle
branch
from
63e76b3 to
4004fbd
Compare
|
Thanks! |
|
Thank you! |
This adds a new optional (but on-by-default) dependency on the
cpp_demanglecrate. When this feature is enabled, if demangling a frame's symbol as a Rust symbol fails, then we attempt to demangle it as a C++ symbol.r? @alexcrichton
I didn't add any tests because I am lazy. Don't hesitate to push back on this if you feel it needs testing.