SFML Move Semantics

[unlight]

Hi guys,

I was hoping to find some information about the status of move-semantic implementation in SFML. I have seen a bit of discussion regarding C++ standard support and plans on the SFML roadmap, but I am not sure if any concrete decisions have been made, or if anyone is already working on it?

My motivation for asking is because I would be more than happy to work on implementing move semantics for SFML, but only if it fits with SFML's current plans and would be taken seriously / reviewed for merging.

Much love!

[Laurent]

There was a big plan to switch code to a modern C++ standard (starting with writing some unit tests to ensure no regression), but obviously no one has enough free time and motivation to start doing it.

Move semantics don't break API compatibility, so this can be done for SFML 2.6 I guess. Let's do it

[unlight]

Alright, I will take this on then 

Before I start, I would just like to clarify my intentions would be to implement explicit move constructor and move assignment operators for classes that inherit from sf::NonCopyable.

Also, is there a place that the SFML devs usually discuss development when required, or is this forum post appropriate enough?

[EDIT]:

Oh gosh, I just noticed the lengthy discussion on sf::NonCopyable. Am I safe to assume we keep sf::NonCopyable and leave it untouched?

https://en.sfml-dev.org/forums/index.php?topic=21781.15

[Laurent]

Am I safe to assume we keep sf::NonCopyable and leave it untouched?

Yes.

[Elias Daler]

Move semantics don't break API compatibility, so this can be done for SFML 2.6 I guess. Let's do it

Hmm... but it requires a move to C++11. This is a huge change, it's not just about API?...

[Laurent]

Hmm... but it requires a move to C++11. This is a huge change, it's not just about API?...

What exactly is a huge change? To stop supporting non-C++11 compilers? I don't think it's that huge

[unlight]

I am currently postponing work on this feature in favour of something that is on the critical path to SFML 2.6. I shall return!

[Nexus]

Keep also the SFML Roadmap thread and its ongoing discussions in mind, when providing additions possibly targeted to SFML 3.

Some things may still change

What exactly is a huge change? To stop supporting non-C++11 compilers? I don't think it's that huge

I was thinking the same here, and maybe this is a part that should be discussed in the roadmap. The biggest risk I see is that SFML 3 is again several years away, and people have to wait even longer for useful additions like move semantics.

Projects that want to do "everything right" at once are often problematic. But as eXpl0it3r mentioned, SFML 3 might be rather short-lived, hoping that we can iterate faster on possible design mistakes.

[unlight]

Alright, I am ready to start working on move semantics again, but a few conversations have happened around the place and I would like to make some suggestions about how to approach the problem.

Non-breaking move semantics
The initial plan is to implement move constructors and assignment operators only, and leave sf::NonCopyable to handle deleted copy functions. This way, the changes can roll straight in without API breakage. I assume that in the future, we may want to instead use =delete instead of sf::NonCopyable during a major update. This will be a much quicker job if the move semantics are already implemented.

Implement on a per-module basis
Implementing move semantics in SFML will not be trivial. The biggest risk is that I start working on it, make decent progress, but stop short due to lack of time or interest. Someone else might also try and do the same, and it becomes an endless ordeal. I suggest that we implement move semantics on a per-module basis. This would be less intimidating to work on and the job could be divided up for others to help with if it interests them.

[Nexus]

You mention some good points. I also think a modular approach makes more sense -- could even be on a class/group of classes basis, to have smaller incremental steps that can be reviewed in reasonable time.

I expect that after the release of SFML 2.6, the master branch will start targeting SFML 3 and new features will be merged against that. eXpl0it3r probably knows the approach in more details here.

So, what's left to decide is really:

are move semantics pressing enough to introduce them into SFML 2.x
or should we prioritize making a development branch available for SFML 3, so that PRs with new features can be processed?

[Xiaohu]

IMHO, the use of new C++ features, and more generally SFML codes changes are of two kinds:

- internal SFML code modifications that do not affect the library users (auto, decltype)
- modifications that affect user code (move semantics, destructuration... No or almost others?)

Of course, it's not that simple but some modifications can be 50/50 or more like in favor of one thing.

With any of the new C++ features, for me, the most important is move semantic and others are generally relevant to internal SFML code. The idea behind that is that internal modification is not critical but external changes or new features are, when the user code should change or, better, when new things are possible that were not possible previously. Use of auto inside of a method doesn't charge more for the user code, but move semantic is way more useful for the user, that can write code that he couldn't do earlier, and it's also only C++11 (so stable feature and no C++14, C++17 or +) so for me, it's the most important feature to implement. In addition, the code modifications needed are probably relatively small, and standard methods can probably be used. The small codes changes should however be tested because move semantics is (litteraly lol) a memory-critical feature

Another argument to implement move semantics: many new C++ features take advantage of move semantics. Of course, with this sort of argument, it's the snake biting its tail, but it should be noticed that even if SFML doesn't use C++11, probably like 90% of users do, and try to implement pre-C++11 code in an environment that probably use all that. So it's critical. And at least the others new features would maybe need to implement move semantics.

So for me: litteraly now ^^ (next release, that is SFML 2.x)

[unlight]

You mention some good points. I also think a modular approach makes more sense -- could even be on a class/group of classes basis, to have smaller incremental steps that can be reviewed in reasonable time.

Adding the move semantics on a  class/group of classes basis is a great idea. As well as making review more managable, it also means more poeple can work on the task in parallel.

are move semantics pressing enough to introduce them into SFML 2.x
or should we prioritize making a development branch available for SFML 3, so that PRs with new features can be processed?

I am not fussed either way, I feel that adding move semantics in SFML 2.x would be in conjunction with sf::NonCopyable, where as SFML 3.0 is an opportunity to ditch sf::NonCopyable altogether. I am not whether a conclusion was ever reached on that discussion.

One last thing I would like to know, what module do you think would be most pressing to work on?

[unlight]

There are two active PRs for move semantics on Github at the moment that have taken different approaches. I think this provides a good opportunity to discuss a method to standardise on.

Shaders
https://github.com/SFML/SFML/pull/1676
Performs move by swapping lhs object state with rhs object state.

Window
https://github.com/SFML/SFML/pull/1681
Performs move by destroying lhs object state, assuming rhs object state, and reinitialising rhs object state with sensible defaults.

I personally think that the first method is more prone to undesired behaviour. For instance, if the move constructor for a window was to perform a straight swap, then both windows would remain open after the operation completes. Any thoughts? Should we push for a consistent approach for move semantics?

[Edit]:
I found a discussion on the topic from SO.
https://stackoverflow.com/questions/6687388/why-do-some-people-use-swap-for-move-assignments

[Nexus]

First, there are different levels of exception guarantees, specifying how involved objects behave in the presence of exceptions:

• strong - every object is always in a semantically valid state
• basic - every object is still destructible and doesn't evoke UB
• none - no guarantees

There are several idiomatic ways to implement the special member functions with C++11, to sum up:

1. Rule Of Five
Overload copy/move ctor, copy/move op= and dtor.
This is the "stupid" approach which requires a big amount of boilerplate, code duplication and thus is rather prone to errors when extending a class.

2. Copy-and-Swap
Overload all 5 special methods + swap(). Use swap() in copy/move op=.
Avoids some code duplication, but still a lot of boilerplate.
Achieves strong exception guarantee (each object is always in a valid state).

3. Rule of Four-and-a-half
Overload copy/move ctor and dtor as usual.
Overload op= taking a value (not const- or rvalue-reference), which means it acts as both move and copy assignments.

4. Rule of Zero
Let the compiler generate the Big Five.
Code complexity wise, this is the best solution, as the behavior is clear and no member can be forgotten. It does not work when there are complex ownership rules, or custom copy logic (for handles).
I've found my smart pointer aurora::CopiedPtr to be a very nice fit for pointers which require deep copies. It can easily reduce the number of methods needed from 5 to 0. For example, in Thor I need it for quite a few things.
Downside: with compiler-generated member-wise copy, the exception guarantee is often reduced to basic (when the copy operation aborts in the middle).

I'm wondering if strong exception guarantee is something we really have to go for in SFML. Generally, I'm more in favor of readable/compact code. Having to implement the Big Five for every 2nd class is also often an indication of bad ownership design or lack of internal abstraction (too many types have custom ownership rules).

[unlight]

I personally don't have a preference for how we approach move semantic support. I agree that manually writing the big 5 for each class is undesirable and I am also in support of concise, readable code. However, I really want behaviour to be logical, and I don't think that swapping internal state for resources like windows during move operations is logical.

A note worth mentioning is that if the internal handles were unique pointers then default move constructors would be perfect. Maybe this is a more valid approach for move semantics in some cases.

Ultimately, I am happy to work on this job in what ever way you more senior members think is best. We just need to be consistent. Please, contribute to the discussion and be harsh on the PRs. I will do the grunt work