Skip to content

Commit

Permalink
Fixed the compilation issue on macOS15 M platform (#252)
Browse files Browse the repository at this point in the history
  • Loading branch information
@monktan89
monktan89 authored Sep 29, 2024
1 parent f8add83 commit 4623101
Showing 1 changed file with 18 additions and 18 deletions.
36 changes: 18 additions & 18 deletions src/Util/util.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,24 @@ int uv_exepath(char *buffer, int *size) {

using namespace std;

#ifndef HAS_CXA_DEMANGLE
// We only support some compilers that support __cxa_demangle.
// TODO: Checks if Android NDK has fixed this issue or not.
#if defined(__ANDROID__) && (defined(__i386__) || defined(__x86_64__))
#define HAS_CXA_DEMANGLE 0
#elif (__GNUC__ >= 4 || (__GNUC__ >= 3 && __GNUC_MINOR__ >= 4)) && \
!defined(__mips__)
#define HAS_CXA_DEMANGLE 1
#elif defined(__clang__) && !defined(_MSC_VER)
#define HAS_CXA_DEMANGLE 1
#else
#define HAS_CXA_DEMANGLE 0
#endif
#endif
#if HAS_CXA_DEMANGLE
#include <cxxabi.h>
#endif

namespace toolkit {

static constexpr char CCH[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
Expand Down Expand Up @@ -583,24 +601,6 @@ bool setThreadAffinity(int i) {
return false;
}

#ifndef HAS_CXA_DEMANGLE
// We only support some compilers that support __cxa_demangle.
// TODO: Checks if Android NDK has fixed this issue or not.
#if defined(__ANDROID__) && (defined(__i386__) || defined(__x86_64__))
#define HAS_CXA_DEMANGLE 0
#elif (__GNUC__ >= 4 || (__GNUC__ >= 3 && __GNUC_MINOR__ >= 4)) && \
!defined(__mips__)
#define HAS_CXA_DEMANGLE 1
#elif defined(__clang__) && !defined(_MSC_VER)
#define HAS_CXA_DEMANGLE 1
#else
#define HAS_CXA_DEMANGLE 0
#endif
#endif
#if HAS_CXA_DEMANGLE
#include <cxxabi.h>
#endif

// Demangle a mangled symbol name and return the demangled name.
// If 'mangled' isn't mangled in the first place, this function
// simply returns 'mangled' as is.
Expand Down

1 comment on commit 4623101

@alexliyu7352
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review for src/Util/util.cpp:

Code Review: Patch to src/Util/util.cpp

Summary

This patch addresses a compilation issue on macOS 15 M by adding a conditional compilation block for HAS_CXA_DEMANGLE. This block ensures that the abi::__cxa_demangle function is only used when available, preventing compilation errors on platforms where it's not supported.

Detailed Feedback

Code Overview

The patch introduces a conditional compilation block to enable HAS_CXA_DEMANGLE only when the __cxa_demangle function is available. This is done by checking for specific compiler versions and platforms.

Strengths

  • Addresses Compilation Issue: The patch effectively resolves the compilation issue on macOS 15 M by preventing the use of abi::__cxa_demangle when it's not supported.
  • Clear and Concise: The conditional compilation block is well-structured and easy to understand.
  • Platform-Specific: The patch correctly identifies the platforms where __cxa_demangle is not supported and disables its use accordingly.

Areas for Improvement

1. Compiler Support

  • Issue: The current conditional compilation block only checks for GCC and Clang compilers. Other compilers, such as Intel C++ Compiler (ICC), might also support __cxa_demangle.
  • Suggestion: Consider adding checks for other compilers that might support __cxa_demangle. This would ensure broader compatibility.
  • Example: 
    #if defined(__ICC) && __ICC >= 1000 // Example for ICC
#define HAS_CXA_DEMANGLE 1
#endif

2. Android NDK Support

  • Issue: The comment mentions that Android NDK support is uncertain.
  • Suggestion: Investigate the current status of __cxa_demangle support in Android NDK. If it's available, update the conditional compilation block accordingly. If not, provide a clear explanation of the limitations and potential workarounds.

3. Documentation

  • Issue: The comment "TODO: Checks if Android NDK has fixed this issue or not." is outdated and should be removed.
  • Suggestion: Update the comments to reflect the current state of __cxa_demangle support on different platforms.

Conclusion

This patch effectively addresses the compilation issue on macOS 15 M. However, it could be improved by adding support for other compilers and clarifying the status of Android NDK support. Updating the documentation would also enhance the code's clarity and maintainability.

TRANS_BY_GITHUB_AI_ASSISTANT

Please sign in to comment.