Skip to content

Conversation

@ChrisThrasher
Copy link
Member

@ChrisThrasher ChrisThrasher commented Sep 2, 2024

Closes #239

This pull request changes CSFML to use SFML's policy for assertions. In short, if a user-provided argument is able to invoke UB, we assert against that potential value. In switching to CSFML to this pattern, we get a few benefits.

  1. Errors are much more visible. Instead of returning a dummy value and logging, we instead terminate the program. Program termination is hard to ignore. This will lead to users of CSFML more quickly finding and fixing their errors.
  2. The implementation is easier to read. These macros from Internal.hpp aren't too hard to understand but it's certainly easier to read an idiomatic assert plus a normal line of code.
  3. More implementation flexibility. These macros assumed the pointer contained a member named This. Now that the macros are gone we can rename those members or pick different implementation techniques like what I proposed in Use inheritance to implement wrapper struct types #222 for example.
  4. Less logging. With Internal.hpp gone CSFML now no longer directly uses sf::err or any other logging utilities. I prefer libraries not do logging without a way to intercept and redirect those logs which CSFML currently does not have.

CSFML as it stands prior to this PR defines the behavior of null pointer arguments in Debug builds while leaving that behavior undefined for Release builds so that aspect of CSFML has not changed. All this PR does is change what that Debug behavior is.

@ChrisThrasher ChrisThrasher merged commit 00f78c1 into master Sep 2, 2024
@ChrisThrasher ChrisThrasher deleted the assertions branch September 2, 2024 17:49
@eXpl0it3r eXpl0it3r added this to the 3.0 milestone Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fail fast on invalid null arguments

3 participants