Skip to content

add support for demangling c++ type names (libstdc++)#578

Open
BerndAmend wants to merge 3 commits into
ChaiScript:developfrom
BerndAmend:feature/demangle-names-for-libstdc++
Open

add support for demangling c++ type names (libstdc++)#578
BerndAmend wants to merge 3 commits into
ChaiScript:developfrom
BerndAmend:feature/demangle-names-for-libstdc++

Conversation

@BerndAmend
Copy link
Copy Markdown
Contributor

Changes proposed in this pull request

  • add support for demangling c++ type names (libstdc++)
  • dump_system(): output demangled names for registered types
  • change get_type_name to return the demangled_name (if no broader type exists)

@lefticus
Copy link
Copy Markdown
Member

@leftibot review this PR and take into consideration the impact it might have across supported platforms

@leftibot
Copy link
Copy Markdown
Contributor

PR Review: Demangling C++ Type Names

Summary

This PR adds abi::__cxa_demangle support for libstdc++/libc++ and exposes it as demangled_name() on Type_Info. It also changes get_type_name() and dump_system() to use demangled names as their fallback when no user-registered name exists.

Cross-Platform Concerns

1. MSVC — no behavioral change, but a missed opportunity

MSVC's std::type_info::name() already returns human-readable names (e.g., "class std::basic_string<char,...>"), so the #else fallback to bare_name() is functionally fine. However, this means the output of get_type_name() will now differ between platforms: libstdc++ will produce std::string while MSVC will produce class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char>>. If any script code or downstream logic compares these strings, it will break across platforms.

2. MinGW — <cxxabi.h> availability is unreliable

MinGW-w64 with libstdc++ does ship <cxxabi.h>, so __has_include(<cxxabi.h>) will be true. However, abi::__cxa_demangle may not link correctly in all MinGW configurations (particularly older ones or those using exception model mismatches). This PR has no link-time check — it relies entirely on the compile-time __has_include, which is insufficient. Given the existing Workaround_Thread_Local_MinGW branch, MinGW edge cases are clearly a known concern for this project.

3. Emscripten

Emscripten provides <cxxabi.h> and abi::__cxa_demangle and should work correctly here.

Code Quality Issues

4. noexcept is incorrect on demangled_name()

The function is marked noexcept but constructs a std::string (which can throw std::bad_alloc) and calls abi::__cxa_demangle (which allocates). If either throws, std::terminate will be called. The existing name() and bare_name() return const char* and are legitimately noexcept — this new function returns std::string by value and should not be noexcept.

5. Return type inconsistency

bare_name() returns const char*; demangled_name() returns std::string. Since get_type_name() already returns std::string, this works, but the Type_Info API is now inconsistent. Worth noting for future maintenance.

6. Memory management is correct but fragile

The free(ret) pattern is correct per the __cxa_demangle contract, but a std::unique_ptr<char, decltype(&std::free)> would be more idiomatic and exception-safe (especially once the incorrect noexcept is removed):

std::unique_ptr<char, decltype(&std::free)> demangled(
    abi::__cxa_demangle(m_bare_type_info->name(), nullptr, nullptr, &status),
    &std::free);
if (demangled) {
    return std::string(demangled.get());
}

7. #if __has_include(...) is used in both the include directive and function body

This is evaluated at compile time in both locations, which is fine, but if someone were to conditionally define __has_include or use a compiler that doesn't support it (pre-C++17), this would fail silently. Since ChaiScript already requires C++17, this is acceptable but worth a brief note: if the project ever adds a compiler that has C++17 features but a broken __has_include (unlikely but not impossible), this would need guarding.

Behavioral Impact

8. get_type_name() fallback change affects error messages and type_name() script function

The change at dispatchkit.hpp:614 (bare_name()demangled_name()) affects every unregistered type's display name — error messages, type_name() in scripts, dump_type() output. This is a user-visible behavior change for anyone who parses or matches on these strings. For registered types, nothing changes (the registered name is returned first).

Recommendation

This PR needs the following before merging:

  1. Remove noexcept from demangled_name() — it is incorrect.
  2. Use RAII for the free()std::unique_ptr with a custom deleter.
  3. Consider adding MSVC demangling via UnDecorateSymbolName or simply document the cross-platform behavioral difference. At minimum, add a note that demangled_name() output is platform-dependent.
  4. Add a test that exercises demangled_name() and/or type_name() for an unregistered type to prevent regressions.
  5. Verify MinGW linking — a __has_include check is not a link-time guarantee.

Without at least items 1 and 2, I would not merge this — the noexcept violation is a latent std::terminate waiting to happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants