Skip to content
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.

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

Create macros for throwing to avoid boilerplate code #151

Merged
merged 23 commits into from
Jan 25, 2025

Conversation

st0rmbtw
Copy link
Contributor

This PR adds a new Throw.h file with the LLGL_THROW and LLGL_THROW_IF macros that work only if the LLGL_DEBUG or the LLGL_ENABLE_EXCEPTIONS are defined. So now LLGL can build with the -fno-exceptions compiler flag.

Copy link
Owner

@LukasBanana LukasBanana left a comment

Choose a reason for hiding this comment

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

I appreciate the cleanup work here. Another jobsite I wanted to pickup a long time ago and only did so incrementally. I'm fine with the new macro in the public interface for the few places with exceptions, like the container classes, but for all internal code, I'd prefer to stick with the LLGL_TRAP/LLGL_ASSERT macros as described in the comments.

include/LLGL/Throw.h Show resolved Hide resolved
include/LLGL/Throw.h Outdated Show resolved Hide resolved
include/LLGL/Throw.h Outdated Show resolved Hide resolved
include/LLGL/Throw.h Outdated Show resolved Hide resolved
include/LLGL/Throw.h Outdated Show resolved Hide resolved
include/LLGL/TypeInfo.h Outdated Show resolved Hide resolved
sources/Platform/Linux/LinuxDisplay.cpp Outdated Show resolved Hide resolved
sources/Renderer/Metal/RenderState/MTGraphicsPSO.mm Outdated Show resolved Hide resolved
@LukasBanana LukasBanana self-assigned this Jan 24, 2025
@LukasBanana LukasBanana added the feature request Requested features and TODO lists label Jan 24, 2025
Copy link
Owner

@LukasBanana LukasBanana left a comment

Choose a reason for hiding this comment

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

I think we can boil this down to just one new macro in the public interface. I left some more comments in the code changes.

include/LLGL/Throw.h Outdated Show resolved Hide resolved
sources/Renderer/Metal/RenderState/MTGraphicsPSO.mm Outdated Show resolved Hide resolved
include/LLGL/TypeInfo.h Outdated Show resolved Hide resolved
sources/Renderer/Metal/MTCore.mm Outdated Show resolved Hide resolved
include/LLGL/Throw.h Outdated Show resolved Hide resolved
@st0rmbtw st0rmbtw requested a review from LukasBanana January 25, 2025 01:42
Copy link
Owner

@LukasBanana LukasBanana left a comment

Choose a reason for hiding this comment

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

I found one last issue, the rest looks good. Once you updated this last bit, I'll merge tomorrow after the CIS build went through.

sources/Renderer/OpenGL/Platform/MacOS/MacOSGLContext.mm Outdated Show resolved Hide resolved
@LukasBanana LukasBanana merged commit 671c74e into LukasBanana:master Jan 25, 2025
47 checks passed
@LukasBanana
Copy link
Owner

I made some changes to the new exception throwing with e8d86d7. I hope you don't feel blindsided, I appreciate you getting the ball rolling on this matter. The behavior hasn't really changed, this will still throw exceptions if enabled, but in the case they are disabled, this provides a nice callstack to the standard error or breaks the debugger if one is attached. The only downside is that this change only accepts a handful of predefined exceptions such as std::invalid_argument, std::bad_cast etc., but I think that's fine since LLGL does handle any exceptions at all, it only throws them which allows the client programmer to exit the application more gracefully than with an abort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Requested features and TODO lists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants