sf::NonCopyable and =delete

[Nexus]

Maybe we should ask ourselves also what kind of move/copy semantics SFML will support, and how corresponding base classes would look. As we all know, C++ is much more gradual than copyable vs. non-copyable now.

• Copyable and movable: The default, all lightweight classes but also certain resources (e.g. sf::Image). Ideally, we could use the Rule Of Zero here, but unfortunately this is not possible with using standard smart pointers alone. How are we going to implement them without boilerplate code? My library Aurora would offer a solution, other ideas?
• Non-copyable, movable: Most if not all of the classes that have been non-copyable before would fall into this category. The main use case are heavy system resources such as windows.
• Non-movable: There is no intrinsic reason why a class should be non-movable, as movability can always be achieved by introducing an indirection. And even without that, it would be more "movable without strong exception guarantee", e.g. for arrays where move implies copy. Do we have such classes?

[Hiura]

I like the current API design, where 'data' resources can be copied (e.g. Image) but 'system' resources cannot (e.g. Window), and I believe we have no classes falling in your third category, nor should we have some. Since you bring the question, do you think we should change something in that respect?

[Nexus]

We should definitely make 'data' resources both copyable and movable, but that's planned anyway.

And as mentioned above, it would be good if we could avoid The Big Five whenever a class needs both copy and move semantics, but unfortunately this is not possible with standard tools alone (as soon as a class doesn't only have value members)...

[Ceylo]

To add my non-constructive ( ) opinion to this old thread:

I'm for using what C++ natively provides because:
- creating sf::Copyable is reinventing the wheel, it's not because you disagree with the standard for what's best that you should do it your own way. A 10 pages argument to explain that it's shorter to read is not worth the benefit of using standard already known stuff.
- modern C++ developers will know these operators, and as years pass, more and more will be used to that standard rather than non_copyable class
- by doing it yourself you prevent yourself from using more subtle cases like: non-copyable & non-movable, movable but non-copyable, plus the possible noexcept specifier on these methods, and eventually the differences that can be introduced with deleting copy constructor but keeping  copy assignement operator available, even if it's probably an unneeded corner case in SFML's usage
- you may miss benefits from C++ evolutions that can make use of the deleted or available operators, which precisely define the allowed behavior, while a sf::NonCopyable would over-specify what's actually needed just because it'd need to be usable commonly in SFML.

[eXpl0it3r]

Someone raised a concern on GitHub that the parent NonCopyable approach still allows for copying, if a copy constructor is explicitly implemented.

#include <iostream>
 
class noncopyable {
public:
        noncopyable() = default;
        noncopyable(const noncopyable&) = delete;
        noncopyable(noncopyable&&) = delete;
        void operator=(const noncopyable&) = delete;
        void operator=(noncopyable&&) = delete;
};
 
class foo : public noncopyable {
public:
        foo() = default;
        foo(const foo&)
        {
                puts("I'm copied!");
        }
};
 
int main() {
        foo f;
        foo f2(f);
        return 0;
}

https://ideone.com/yGp7SP

[Laurent]

Is that a concern? sf::NonCopyable is just a shortcut/convenience to avoid defining all the stuff that makes a class non-copyable. It was never meant to enforce full non-copyability over all levels of derived classes.

If you inherit from a non-copyable class, but define a copy constructor because your derived class knows how to copy itself (without copying the base, obviously), then it's perfectly fine. I've done this many times with Qt and its non-copyable QObject base.

[markand]

IMHO, a parent class for removing/adding functionality in a subclass is not the good way to do. Especially as explained above. Class inheritance means "IS-A" not "HAS"/"HAS NOT".

[Laurent]

Class inheritance means "IS-A" not "HAS"/"HAS NOT".

Public inheritance means "IS-A". Private inheritance, which is how sf::NonCopyable is meant to be used, is like composition (you inherit features, not a public interface), so it really is "HAS".

[nogoodname]

Going to give my 2 cents on this old thread. IMO the concept of NonCopyable isn't as useful as it was in C++03, it now split into 2 concepts now: MoveOnly and NonMoveable.

I'll say that NonCopyable is kinda redundant giving that you can to the same thing with =delete.
E.g. Foo& operator=(Foo&&) = delete; will do the same thing as Foo inheriting from sf::NonCopyable in 1 line.

[reconn]

I believe sf::NonCopyable does more harm than good in the long run. Early/inexperienced developers will definitely inspect SFML code and get inspired by the idioms they see here. I think SFML should continue promoting good C++.

That said, seeing sf::NonCopyable - how simple, useful and concise it is - some may think it's a great idea. But sf::NonCopyable is in fact a code smell, syntactic sugar. The class can be inherited publicly which causes trouble with polymorphic destruction of base classes, and even the documentation says public inheritance does not matter here:

/// The type of inheritance (public or private) doesn't matter,
/// the copy constructor and assignment operator are declared private
/// in sf::NonCopyable so they will end up being inaccessible in both
/// cases.

To be safe it would require a virtual destructor which imposes an additional cost. Sure, private inheritance is safe but it's still ambiguous when somebody inherits directly from NonCopyable but provides a copy constructor or adds default move constructor/assignment anyways which separates statements of the move "=delete"s from NonCopyable.

All that mess for questionable readability/simplicity.