Skip to content

Alternative to rtti for the event bus. #1942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 8, 2025
Merged

Alternative to rtti for the event bus. #1942

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 23 additions & 6 deletions src/support/eventbus.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,34 @@ SOFTWARE.
#include <any>
#include <functional>
#include <memory>
#include <typeindex>
#include <typeinfo>
#include <utility>

#include "support/hashtable.h"
#include "support/list.h"

#if defined(_CPPRTTI) || defined(__GXX_RTTI) || defined(__cpp_rtti)
#define PCSX_RTTI 1
#include <typeindex>
#include <typeinfo>
#define PCSX_HASH_TYPE(type) typeid(type).hash_code()
#else
#define PCSX_RTTI 0
#define CTTI_HAS_ENUM_AWARE_PRETTY_FUNCTION 1
#define CTTI_HAS_VARIABLE_TEMPLATES 1
#include "ctti/type_id.hpp"
#define PCSX_HASH_TYPE(type) ctti::unnamed_type_id<type>().hash()
Comment on lines +44 to +47
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify CTTI library configuration and compatibility.

The CTTI library configuration defines are hardcoded. Ensure these settings are compatible with your target environments and the version of CTTI being used.

🏁 Script executed:

#!/bin/bash
# Description: Check if CTTI library is properly integrated and verify the hash return type

# Check if CTTI headers exist in the project
fd -t f -e hpp -e h "type_id" | head -10

# Look for CTTI-related build configuration
rg -i "ctti" --type cmake --type make | head -5

# Verify the hash method return type in CTTI documentation or source
rg -A 5 -B 5 "hash.*uint64" | head -10

Length of output: 18316

CTTI Library Integration and Compatibility Needs Attention

  • No ctti/type_id.hpp (or other CTTI headers) were found in the repository; confirm the CTTI library is added as a dependency (e.g. submodule or external package) and that your build system (CMake/Makefiles) includes its header directory.
  • There are no CTTI-related entries in CMakeLists.txt or Makefiles—add the necessary find_package or include-path settings so the compiler can locate the headers.
  • The hardcoded macros 
    #define CTTI_HAS_ENUM_AWARE_PRETTY_FUNCTION 1
#define CTTI_HAS_VARIABLE_TEMPLATES 1
    must match the actual CTTI version you’re using. Verify your version supports these features before defining them.
  • Ensure that ctti::unnamed_type_id<type>().hash() returns the intended width (e.g., std::size_t or uint64_t) on all target platforms. If you depend on a 64-bit hash, guard or static-assert accordingly.
🤖 Prompt for AI Agents 
In src/support/eventbus.h around lines 44 to 47, verify that the CTTI library is
properly integrated by ensuring its headers are present in the project and that
your build system (CMake or Makefiles) includes the correct include paths or
find_package directives for CTTI. Confirm the hardcoded macros
CTTI_HAS_ENUM_AWARE_PRETTY_FUNCTION and CTTI_HAS_VARIABLE_TEMPLATES match the
actual CTTI version used, adjusting or removing them if incompatible.
Additionally, check that ctti::unnamed_type_id<type>().hash() returns the
expected hash width on all target platforms, adding static assertions or guards
if a specific size like 64-bit is required.

#endif

namespace PCSX {

namespace EventBus {

#if PCSX_RTTI
typedef std::size_t HashType;
#else
typedef std::uint64_t HashType;
#endif

struct ListenerElementBaseEventBusList {};
struct ListenerElementBase;
typedef PCSX::Intrusive::List<ListenerElementBase> ListenerBaseListType;
Expand Down Expand Up @@ -70,7 +87,7 @@ class Listener {
};

struct ListenerElementsHashTableList;
typedef PCSX::Intrusive::HashTable<std::size_t, ListenerElementsHashTableList> ListenersHashTable;
typedef PCSX::Intrusive::HashTable<HashType, ListenerElementsHashTableList> ListenersHashTable;
struct ListenerElementsHashTableList : public ListenersHashTable::Node {
~ListenerElementsHashTableList() { list.destroyAll(); }
ListenerBaseEventBusList list;
Expand All @@ -82,7 +99,7 @@ class EventBus {
template <typename Event>
void signal(const Event& event) {
using Functor = typename ListenerElement<Event>::Functor;
auto list = m_table.find(typeid(Event).hash_code());
auto list = m_table.find(PCSX_HASH_TYPE(Event));
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Consider potential ABI compatibility implications.

The macro usage is correct and maintains a clean interface. However, switching between RTTI and non-RTTI modes could break ABI compatibility since the hash types and algorithms differ.

Consider documenting the ABI implications of this change and ensure that:

  1. All translation units use the same RTTI mode
  2. The build system enforces consistent RTTI settings
  3. Different hash algorithms don't introduce subtle behavioral differences

Also applies to: 112-112, 127-127

🏁 Script executed:

#!/bin/bash
# Description: Check for potential mixed RTTI usage patterns in the codebase

# Look for direct typeid usage that might bypass the macro
rg "typeid\(" --type cpp --type cc --type cxx | grep -v "PCSX_HASH_TYPE"

# Check for existing RTTI-dependent code that might conflict
rg -i "rtti" --type cpp --type cc --type cxx --type cmake | head -10

# Look for build configuration that might affect RTTI settings
rg -i "frtti\|fno-rtti" --type cmake --type make

Length of output: 246

The previous ripgrep invocation failed to detect “.cc” files, so let’s retry with explicit glob patterns:

🏁 Script executed:

#!/bin/bash
# 1) Find any direct typeid() usages not wrapped in PCSX_HASH_TYPE
rg "typeid\\(" -g '*.cpp' -g '*.cc' -g '*.cxx' --no-heading | grep -v "PCSX_HASH_TYPE"

# 2) Scan for any RTTI mentions in source, headers, and CMake
rg -i rtti -g '*.cpp' -g '*.cc' -g '*.cxx' -g '*.h' -g '*.hpp' -g '*.cmake' -g 'CMakeLists.txt' | head -20

# 3) Look for RTTI-related compiler flags in CMakeLists
rg -i 'fno-rtti|frtti' -g '*.cmake' -g 'CMakeLists.txt' --no-heading

Length of output: 2188

Document ABI implications and enforce RTTI consistency

No direct typeid() calls bypassing PCSX_HASH_TYPE were found in your sources (only in third_party/EABase tests). However, the build system doesn’t currently lock down RTTI settings, which risks mixing RTTI modes and breaking ABI. To address this:

  • In src/support/eventbus.h, expand the comment around PCSX_RTTI to explain that it switches hash algorithms and impacts the ABI.
  • In your top‐level CMakeLists.txt (and any per‐module CMake files), add compiler flags (-frtti or -fno-rtti) uniformly so all targets share the same RTTI mode.
  • Consider a CMake configuration check that errors out if different targets compile with mismatched RTTI settings.
🤖 Prompt for AI Agents 
In src/support/eventbus.h at line 102, update the comment near the
PCSX_HASH_TYPE(Event) macro usage to explicitly document that switching between
RTTI and non-RTTI modes changes the hash algorithm and affects ABI
compatibility. Additionally, ensure the top-level CMakeLists.txt and any
module-specific CMake files enforce consistent RTTI compiler flags (-frtti or
-fno-rtti) across all targets, and implement a CMake configuration check that
fails the build if RTTI settings differ between targets to prevent ABI breakage.

if (list == m_table.end()) return;
for (auto& listener : list->list) {
std::any cb = listener.getCB();
Expand All @@ -92,7 +109,7 @@ class EventBus {
}

private:
void listen(std::size_t id, ListenerElementBase* listenerElement) {
void listen(HashType id, ListenerElementBase* listenerElement) {
auto list = m_table.find(id);
if (list == m_table.end()) {
list = m_table.insert(id, new ListenerElementsHashTableList());
Expand All @@ -107,7 +124,7 @@ template <typename Event>
void Listener::listen(typename ListenerElement<Event>::Functor&& cb) {
ListenerElement<Event>* element = new ListenerElement(std::move(cb));
m_listeners.push_back(element);
m_bus->listen(typeid(Event).hash_code(), element);
m_bus->listen(PCSX_HASH_TYPE(Event), element);
}

} // namespace EventBus
Expand Down
Loading