Skip to content

Add builtin_unreachable to the FW_ASSERT macros #5228

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

Open
garthwatney wants to merge 5 commits into
base: devel
Choose a base branch
from
+43 −11
Open

Add builtin_unreachable to the FW_ASSERT macros #5228

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d04aae0
Update Assert.hpp
garthwatney Jun 2, 2026
2ec0abb
Add FW_UNREACHABLE macro for better compatibility
thomas-bc Jun 2, 2026
469d891
Refactor FW_ASSERT_UNREACHABLE macro definition
thomas-bc Jun 12, 2026
f293c95
Refactor FW_UNREACHABLE definition for better compatibility
thomas-bc Jun 12, 2026
1468516
Guard ifndef
thomas-bc Jun 12, 2026
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
52 changes: 41 additions & 11 deletions Fw/Types/Assert.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,33 @@
// Return all the arguments of the macro, but the first one
#define FW_ASSERT_NO_FIRST_ARG(ARG_0, ...) __VA_ARGS__

// Define FW_UNREACHABLE to hint to the compiler that a code path is unreachable.
Comment thread
thomas-bc marked this conversation as resolved.
// Falls back to a no-op on compilers that do not support __builtin_unreachable.
// (1) __has_builtin — the modern check, available in Clang and GCC >= 10.
// (2) defined(__GNUC__) — catches older GCC (4.5–9) which lack __has_builtin
#ifdef __has_builtin
#if __has_builtin(__builtin_unreachable)
#define FW_UNREACHABLE() __builtin_unreachable()
Comment thread
thomas-bc marked this conversation as resolved.
#endif
#endif
Comment on lines +11 to +19

@lestarch-autobot lestarch-autobot Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The Copilot review correctly identified that this branch can redefine a user-provided FW_UNREACHABLE. Other macros in this file (e.g., FW_ASSERT_NORETURN at line 88, NOINLINE at line 102) use #ifndef guards. Wrapping the __has_builtin detection in #ifndef FW_UNREACHABLE prevents redefinition warnings and allows intentional overrides.

Suggested change
// Define FW_UNREACHABLE to hint to the compiler that a code path is unreachable.
// Falls back to a no-op on compilers that do not support __builtin_unreachable.
// (1) __has_builtin — the modern check, available in Clang and GCC >= 10.
// (2) defined(__GNUC__) — catches older GCC (4.5–9) which lack __has_builtin
#ifdef __has_builtin
#if __has_builtin(__builtin_unreachable)
#define FW_UNREACHABLE() __builtin_unreachable()
#endif
#endif
// Define FW_UNREACHABLE to hint to the compiler that a code path is unreachable.
// Users may pre-define FW_UNREACHABLE to override the detection below.
// Falls back to a no-op on compilers that do not support __builtin_unreachable.
// (1) __has_builtin — the modern check, available in Clang and GCC >= 10.
// (2) defined(__GNUC__) — catches older GCC (4.5–9) which lack __has_builtin
#ifndef FW_UNREACHABLE
#ifdef __has_builtin
#if __has_builtin(__builtin_unreachable)
#define FW_UNREACHABLE() __builtin_unreachable()
#endif
#endif
#endif

#ifndef FW_UNREACHABLE
#if defined(__GNUC__)
#define FW_UNREACHABLE() __builtin_unreachable()
#else
#define FW_UNREACHABLE() ((void)0) // no-op fallback for unknown compilers
#endif
#endif
Comment thread
thomas-bc marked this conversation as resolved.

// FW_ASSERT_UNREACHABLE is the hint used inside FW_ASSERT macros.
// Only active when FW_ASSERTIONS_ALWAYS_ABORT is enabled, because SwAssert
// can legally return when it is disabled (the default). Using __builtin_unreachable
// after a call that returns is undefined behavior.
#if FW_ASSERTIONS_ALWAYS_ABORT
Comment thread
thomas-bc marked this conversation as resolved.
#define FW_ASSERT_UNREACHABLE() FW_UNREACHABLE()
#else
#define FW_ASSERT_UNREACHABLE() ((void)0)
#endif

#if FW_ASSERT_LEVEL == FW_NO_ASSERT
// Users may override the NO_ASSERT case should they choose
#ifndef FW_ASSERT
Expand All @@ -23,24 +50,27 @@
#define FW_ASSERT(...) \
((FW_ASSERT_FIRST_ARG(__VA_ARGS__, 0)) \
? ((void)0) \
: (Fw::SwAssert(ASSERT_FILE_ID, FW_ASSERT_NO_FIRST_ARG(__VA_ARGS__, __LINE__))))
: (Fw::SwAssert(ASSERT_FILE_ID, FW_ASSERT_NO_FIRST_ARG(__VA_ARGS__, __LINE__)), FW_ASSERT_UNREACHABLE()))
#elif FW_ASSERT_LEVEL == FW_FILEID_ASSERT && !defined ASSERT_FILE_ID
#define FILE_NAME_ARG U32
#define FW_ASSERT(...) \
((FW_ASSERT_FIRST_ARG(__VA_ARGS__, 0)) \
? ((void)0) \
: (Fw::SwAssert(static_cast<U32>(0), FW_ASSERT_NO_FIRST_ARG(__VA_ARGS__, __LINE__))))
#define FW_ASSERT(...) \
((FW_ASSERT_FIRST_ARG(__VA_ARGS__, 0)) \
? ((void)0) \
: (Fw::SwAssert(static_cast<U32>(0), FW_ASSERT_NO_FIRST_ARG(__VA_ARGS__, __LINE__)), \
FW_ASSERT_UNREACHABLE()))
#elif FW_ASSERT_LEVEL == FW_RELATIVE_PATH_ASSERT && defined ASSERT_RELATIVE_PATH
#define FILE_NAME_ARG const CHAR*
#define FW_ASSERT(...) \
((FW_ASSERT_FIRST_ARG(__VA_ARGS__, 0)) \
? ((void)0) \
: (Fw::SwAssert(ASSERT_RELATIVE_PATH, FW_ASSERT_NO_FIRST_ARG(__VA_ARGS__, __LINE__)), \
FW_ASSERT_UNREACHABLE()))
#else
#define FILE_NAME_ARG const CHAR*
#define FW_ASSERT(...) \
((FW_ASSERT_FIRST_ARG(__VA_ARGS__, 0)) \
? ((void)0) \
: (Fw::SwAssert(ASSERT_RELATIVE_PATH, FW_ASSERT_NO_FIRST_ARG(__VA_ARGS__, __LINE__))))
#else
#define FILE_NAME_ARG const CHAR*
#define FW_ASSERT(...) \
((FW_ASSERT_FIRST_ARG(__VA_ARGS__, 0)) ? ((void)0) \
: (Fw::SwAssert(__FILE__, FW_ASSERT_NO_FIRST_ARG(__VA_ARGS__, __LINE__))))
: (Fw::SwAssert(__FILE__, FW_ASSERT_NO_FIRST_ARG(__VA_ARGS__, __LINE__)), FW_ASSERT_UNREACHABLE()))
#endif
#endif // if ASSERT is defined

Expand Down
Loading