Testable debug run-time lifetime tracking

[vittorioromeo]

Add a new SFML/System/LifetimeTracking.hpp header that provides testable run-time lifetime tracking.

---

The tracking is enabled only when SFML_ENABLE_LIFETIME_TRACKING is defined. That macro is controlled by a CMake option of the same name, enabled by default when building in debug mode.

The tracking is also enabled for our CI test suite, regardless of the build mode.

---

The tracking catches common lifetime mistakes between dependee types (e.g. Texture) and dependant types (e.g. Sprite) at run-time, providing the user with a readable error message:

FATAL ERROR: a texture object was destroyed while existing sprite objects depended on it.

Please ensure that every texture object outlives all of the sprite objects associated with it,
otherwise those sprites will try to access the memory of the destroyed texture, 
causing undefined behavior (e.g., crashes, segfaults, or unexpected run-time behavior).
 
One of the ways this issue can occur is when a texture object is created as a local variable 
in a function and passed to a sprite object. When the function has finished executing, the 
local texture object will be destroyed, and the sprite object associated with it will now be
referring to invalid memory. Example:

    sf::Sprite createSprite()
    {
        sf::Texture texture(/* ... */);
        sf::Sprite sprite(texture, /* ... */);

        return sprite;
        //     ^~~~~~
        // ERROR: `texture` will be destroyed right after
        //        `sprite` is returned from the function!
    }

Another possible cause of this error is storing both a texture and a sprite together in a
data structure (e.g., `class`, `struct`, container, pair, etc...), and then moving that 
data structure (i.e., returning it from a function, or using `std::move`) -- the internal 
references between the texture and sprite will not be updated, resulting in the same 
lifetime issue.

In general, make sure that all your texture objects are destroyed *after* all the 
sprite objects depending on them to avoid these sort of issues.

Note that the error message generation dynamically changes depending on the object types, it's not hardcoded for sprites and textures specifically.

---

Adding lifetime tracking to SFML types is trivial and requires minimum changes, as shown by the diff. It consists in a few simple steps:

• In the dependee type (e.g. Texture):

  1. Forward-declare the dependent type (e.g. Sprite);
  2. Add the following code at the end of the dependee type's private section:

  SFML_DEFINE_LIFETIME_DEPENDEE(Texture, Sprite);

• In the dependant type (e.g. Sprite):

  1. Add the following code at the end of the dependant type's private section:

  SFML_DEFINE_LIFETIME_DEPENDANT(Texture);

  2. Add the following code in any function that changes the dependant type's pointer-to-dependee:

  SFML_UPDATE_LIFETIME_DEPENDANT(Texture, Sprite, m_texture);

That's it!

---

For testing, use the provided sf::priv::LifetimeDependee::TestingModeGuard:

SECTION("Return local from function")
{
    const auto badFunction = []
    {
        const auto texture = sf::Texture::create({64, 64}).value();
        return sf::Sprite(texture);
    };

    const sf::priv::LifetimeDependee::TestingModeGuard guard;
    CHECK(!guard.fatalErrorTriggered());

    badFunction();

    CHECK(guard.fatalErrorTriggered());
}

---

Drawbacks:

• Some valid constructs might need to be slightly rearranged to please the lifetime tracker. E.g.:

  // Techincally valid, but triggers a fatal error
  {
      sf::Sprite sprite(texture);
      const sf::Texture otherTexture = sf::Texture::create({64, 64}).value();
      sprite.setTexture(otherTexture);
  }
  
  // Valid, and pleases the lifetime tracker
  {
      const sf::Texture otherTexture = sf::Texture::create({64, 64}).value();
  
      sf::Sprite sprite(texture);
      sprite.setTexture(otherTexture);
  }

  However, this is more of a design problem with sf::Sprite itself than with the lifetime tracker, as sf::Sprite only needs a reference to the texture during its draw call, but SFML artificially extends the relationship between sf::Sprite and sf::Texture way beyond that. See sf::Sprite does not store sf::Texture* #3072 and sf::Sprite as temporary view over geometry + texture #3080 for an ample discussion on that.

---

FAQ:

• Is this useful for newbies?

  • Yes. Newbies might not be familiar with the concepts of lifetime and ownership in C++, and might inadvertedly get into a situation where a dependee is destroyed before a dependant. This PR ensures that they will be alerted of the issue as soon as possible, saving them minutes or hours of frustration, debugging, saving us from having to help them out on Discord, and prompting them to learn more about lifetimes and ownership in C++.

• Is this useful for power users?

  • Yes. While powerusers are familiar with lifetimes and don’t typically struggle with them, they can still make mistakes. This PR aims to alert powerusers to subtle lifetime issues that might arise from complex interactions, refactoring, or occasional human errors. It's not about teaching lifetimes but providing a safety net for those rare mistakes.

• Is there any overhead if lifetime tracking is disabled?

  • Nope. Disabled lifetime tracking incurs no memory or run-time overhead at all.

[Bambo-Borris]

I find this an interesting idea, my only concern is that powerusers aren't the ones with lifetime woes, and noobs may not reap the benefits because lifetimes are such a new concept that they won't think to reach for a debugging tool oriented around it. And also even with nice friendly documentation it will still be underutilised by the audience most likely to need it.

I also worry the verbose error message may be failing to get across the kind of issue the user is facing. Terms like dependants and dependencies are alien to newbies. I myself find the example error message a bit tricky to follow and I'm an experienced developer and one who has used SFML for many years and understands the pitfalls with lifetimes between sprites & textures.

Feels like maybe missing the mark by being more complex than is necessary for the audience of people most likely to require this. I can tell you all the powerusers I speak to don't struggle with texture-sprite lifetime woes, but I regularly see newbies that do. I'd start, at a minimum, with reconsidering the language in the error messages. Maybe even defining what audience you intend to utilise such a feature.

[vittorioromeo]

@Bambo-Borris: Thanks for the feedback!

You raised three main points:

1. The error message could be improved.
2. Newbies might not benefit since they're not familiar with lifetimes.
3. Powerusers don’t struggle with lifetimes.

Here are my thoughts:

1. I agree the error message has room for improvement. We could use simpler language tailored to beginners, include a code example, or link to resources like the "white square problem" page on the SFML website. This could make the message more accessible and helpful for newbies.

2. By tweaking the language of the error message to be newbie-friendly, avoiding terms like "dependee" or "dependant", and providing clear examples or links, I believe beginners will find the message valuable and easier to understand.

3. While powerusers are familiar with lifetimes and don’t typically struggle with them, they can still make mistakes. This PR aims to alert powerusers to subtle lifetime issues that might arise from complex interactions, refactoring, or occasional human errors. It's not about teaching lifetimes but providing a safety net for those rare mistakes.

[coveralls]

Pull Request Test Coverage Report for Build 9517208803

Details

• 92 of 115 (80.0%) changed or added relevant lines in 8 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.1%) to 56.312%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/SFML/System/LifetimeTracking.cpp	69	92	75.0%

Files with Coverage Reduction	New Missed Lines	%
include/SFML/Audio/Sound.hpp	1	0.0%

Totals
Change from base Build 9516560962:	0.1%
Covered Lines:	11813
Relevant Lines:	19859

---

💛 - Coveralls

[vittorioromeo]

New error message example:

FATAL ERROR: a texture object was destroyed while existing sprite objects depended on it.

Please ensure that every texture object outlives all of the sprite objects associated with it,
otherwise those sprites will try to access the memory of the destroyed texture, 
causing undefined behavior (e.g., crashes, segfaults, or unexpected run-time behavior).
 
One of the ways this issue can occur is when a texture object is created as a local variable 
in a function and passed to a sprite object. When the function has finished executing, the 
local texture object will be destroyed, and the sprite object associated with it will now be
referring to invalid memory. Example:

    sf::Sprite createSprite()
    {
        sf::Texture texture(/* ... */);
        sf::Sprite sprite(texture, /* ... */);

        return sprite;
        //     ^~~~~~

        // ERROR: `texture` will be destroyed right after
        //        `sprite` is returned from the function!
    }

Another possible cause of this error is storing both a texture and a sprite together in a
data structure (e.g., `class`, `struct`, container, pair, etc...), and then moving that 
data structure (i.e., returning it from a function, or using `std::move`) -- the internal 
references between the texture and sprite will not be updated, resulting in the same 
lifetime issue.

In general, make sure that all your texture objects are destroyed *after* all the 
sprite objects depending on them to avoid these sort of issues.

Note that the error message generation dynamically changes depending on the object types, it's not hardcoded for sprites and textures specifically.

@Bambo-Borris: What do you think?

[coveralls]

Pull Request Test Coverage Report for Build 9529431118

Details

• 80 of 114 (70.18%) changed or added relevant lines in 8 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.06%) to 56.263%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/SFML/System/LifetimeTracking.cpp	69	103	66.99%

Files with Coverage Reduction	New Missed Lines	%
include/SFML/Audio/Sound.hpp	1	0.0%

Totals
Change from base Build 9516560962:	0.06%
Covered Lines:	11803
Relevant Lines:	19860

---

💛 - Coveralls

[ChrisThrasher]

The tests currently make no mention of the priv:: namespace and I'd like to keep it that way. There are definitely some sf::priv:: things I wish we could test but I'm rather dogmatic about the unit tests only using the public API and am not comfortable testing implementation details like this which users are not intended to use or even know about.

How much code could be removed from this PR if TestingModeGuard was removed?

This code will still be tested although instead of explicitly testing for failure we will be implicitly testing for the lack of failure. I know that's not the same but by simply using types like sf::Sprite we will still be testing that your lifetime tracking has no false positives. We cannot test for true negatives quite as well though but I think I'm okay with that. It's the same as all of our asserts. We can't test for assertion failure but we can test that assertions don't have false positives. In practice this seems to work quite well.

[ChrisThrasher]

sf::Sound and sf::SoundBuffer already perform their own internal lifetime checks where sound buffers are de-registered with the Sounds that use them when the SoundBuffer is destroyed. Do these new lifetime checks means we can remove all that machinery from Sound and SoundBuffer?

[vittorioromeo]

@ChrisThrasher @eXpl0it3r @Bambo-Borris example of lifetime issue that this PR would have caught: https://discord.com/channels/175298431294636032/175298431294636032/1262381178523222086