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

Remove usages of std::auto_ptr to get rid of warnings when building with gcc 8.2 #1546

Closed
wants to merge 1 commit into from

Conversation

therocode
Copy link
Contributor

@therocode therocode commented Jan 18, 2019

Remove usages of std::auto_ptr to get rid of warnings when building with gcc 8.2

  • Has this change been discussed on the forum or in an issue before?
  • Does the code follow the SFML Code Style Guide?
  • Have you provided some example/test code for your changes?

Description

Minor change to get rid of deprecated std::auto_ptr<> usage.

With newer versions of GCC (8.2), building SFML emits the following warning:
warning: 'template class std::auto_ptr' is deprecated

It could be argued that the warning is harmless, but it is important to maintain a warning-free build as many people use -Werror in their projects and this can cause inconvenience when using the library or loss of trust as warnings simply look like there might be issues.

In this pull request I have removed this warning by removing auto_ptr usage. The only two usages of auto_ptr are very trivial - it is used as a way to create a dynamically allocated single instance that is temporary and is destroyed when going out of scope. I replaced this usage with std::vector<> that is set to contain one instance only.
I agree that the usage of std::vector for this is a bit of a hack since we're only ever in need of a single instance, not a list. But I deem that it is worth it since it gets rid of the warning and is semantically equivalent. Ideally this should be a std::unique_ptr<> which can easily be done whenever SFML makes the move for C++11. Until then, this suffices as a hack to get rid of warnings when building with recent compilers (auto_ptr is even removed in C++17 so any conforming compiler would actually render SFML incapable of building when C++17 mode is enabled).

Other solutions possible:
One could make an sf::AutoPtr which basically clones the (bad) behaviour of auto_ptr. I do not advocate for this since it adds another type to SFML which has to be maintained, documented and kept up to standard. It also seems overkill for such a localised issue as the one above.
One could also solve the issue with manual memory management using raw pointers but this is problematic since you need to ensure strong exception safety - if any of the lines of code in the function throw, then we're in for a leak.

All in all, I think this little hack is a cheap price to pay to keep SFML compatible with newer C++ compilers.

Since this code is so trivial, it is easy to see that the code semantics is identical, and testing it is a bit tricky since the functions involved are private. I still tested at least one of the functions by running this code:

int main()
{
    auto res = sf::SoundRecorder::isAvailable();
    std::cout << "res: " << res << "\n";
}

isAvailable() uses the AudioDevice::isExtensionSupported function internally. This code prints 1 for me without any issues on linux. Since the change is identical in both functions, and again, identical to the previous code in semantics, I think the change should be OK.

Tasks

  • Tested on Linux
  • Tested on Windows
  • Tested on macOS
  • Tested on iOS
  • Tested on Android

How to test this PR?

As described above, I tested that the involved code in isExtensionSupported still works by using the following code:

#include <SFML/Audio.hpp>
#include <iostream>

int main()
{
    auto res = sf::SoundRecorder::isAvailable();
    std::cout << "res: " << res << "\n";
}
//should print 1 or 0 depending on if sound recording is available or not

It might not be the best way to test things, and I am open to other suggestions.

@eXpl0it3r eXpl0it3r added this to Discussion in SFML 2.6.0 via automation Jan 19, 2019
@eXpl0it3r eXpl0it3r added this to the 2.6 milestone Jan 19, 2019
@eXpl0it3r eXpl0it3r moved this from Discussion to Review & Testing in SFML 2.6.0 Jan 19, 2019
@eXpl0it3r eXpl0it3r moved this from Review & Testing to Ready in SFML 2.6.0 Jan 19, 2019
@eXpl0it3r
Copy link
Member

Merged in bf92efe

@eXpl0it3r eXpl0it3r closed this Jan 23, 2019
SFML 2.6.0 automation moved this from Ready to Done Jan 23, 2019
@SFML SFML deleted a comment from eXpl0it3r Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
SFML 2.6.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants