Added support for swift demangling #38
Conversation
Corrected mistake in cmake file.
| @@ -38,6 +39,7 @@ Demangler::Demangler() | |||
| { | |||
| loadDemangleLib(QStringLiteral("rustc_demangle"), "rustc_demangle", QByteArrayLiteral("_R")); | |||
| loadDemangleLib(QStringLiteral("d_demangle"), "demangle_symbol", QByteArrayLiteral("_D")); | |||
| m_demanglers.push_back({"$s", reinterpret_cast<Demangler::demangler_t>(swift_demangle)}); | |||
There was a problem hiding this comment.
why is this not using loadDemangleLib like the above?
There was a problem hiding this comment.
swift_demangle can be compiled as a standalone shared library and used the same way as you use rustc_demangle or d_mangle.
There was a problem hiding this comment.
this does not answer my question. can we or can we not reuse loadDemangleLib? if not, why not?
app/swift_demangle.h
Outdated
| #ifndef SWIFT_DEMANGLER_H | ||
| #define SWIFT_DEMANGLER_H | ||
|
|
||
| int swift_demangle(const char* symbol, char* buffer, size_t bufferLength) { |
| // Construct the full path to libswiftDemangle.so: | ||
| // SOMEPATH/usr/lib/libswiftDemangle.so | ||
| char lib_path[PATH_MAX]; | ||
| snprintf(lib_path, sizeof(lib_path), "%s/usr/lib/libswiftDemangle.so", base_path); |
There was a problem hiding this comment.
this seems like it's replicating parts of what we already do for the other demanglers, why can we not integrate with that code? why is the fancy logic to find swift needed, can't we just use normal library lookup instead?
There was a problem hiding this comment.
I thought that the upside of doing it this way was to make it future proof as swift seems still to be evolving fairly rapidly. The downside is that it requires swift to be in the PATH, but only for people that are interested in swift demangling. Other developers are not affected. Also I wasn't sure about licensing issues with distributing a binary library libswiftDemangle.so which is Apache licensed in a GPL project.
There was a problem hiding this comment.
IANAL but linking gpl to apache is fine but not the other way around?
still, you haven't answered my question - PATH has no impact on the dynamic library loader on linux - or are you saying this proposed approach here needs it? I would instead rather we try to dlopen libswiftDemangle.so directly, just like we do for rustc_demangle e.g. unless this isn't possible (why?) please simplify your patch accordingly.
Updated copyright notice
Updated copyright notice
|
please also write clean commits, i.e. use |
Added support for swift demangling. It depends on 'swift' being in the current PATH and will locate libswiftDemangle.so to do the actual demangling. No additional shared library needs to be built or included in 'hotspot' to support demangling and if 'libswiftDemangle.so' is not found then swift demangling is just disabled.