Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: sf::NonCopyable and =delete  (Read 3313 times)

0 Members and 1 Guest are viewing this topic.

eXpl0it3r

  • Moderator
  • Hero Member
  • *****
  • Posts: 9221
    • View Profile
    • development blog
    • Email
sf::NonCopyable and =delete
« on: April 04, 2017, 02:48:34 pm »
Info & Rules about this sub-forum

(Originally posted on the C++ standards thread.)

C++03 didn't offer a way to tell the compiler that a class wasn't meant to be copied, but in order to trick the compiler into preventing exactly that, one could make the copy-constructor and assignment-operator private.
sf::NonCopyable used that trick and by acting as an abstract base class (interface), all classes that should be non-copyable could simple derive from it.

The question would be:
  • Should we keep sf::NonCopyable and just change its implementation to use C++11 = delete?
  • Or should we remove the class and use the native C++11 language feature = delete?
In binary1248's latest C++11 refactoring, he's removed the class, in case you want to see how such a change would look like.


Personally, I find that sf::NonCopyable only existed to trick the compiler for a missing language feature. With C++ we got that feature through = delete, so I don't really see the reason to keep it.

Error messages will change, but people who are used to C++11, should understand the error message of C++11 better than a more cryptic error in combination with sf::NonCopyable.
« Last Edit: April 06, 2017, 08:43:00 pm by eXpl0it3r »
Official FAQ: https://www.sfml-dev.org/faq.php
Nightly Builds: https://www.nightlybuilds.ch/
——————————————————————
Dev Blog: https://dev.my-gate.net/
Thor: http://www.bromeon.ch/libraries/thor/

Rosme

  • Full Member
  • ***
  • Posts: 150
  • Proud member of the shoe club
    • View Profile
    • Code-Concept
Re: sf::NonCopyable and =delete
« Reply #1 on: April 04, 2017, 03:44:15 pm »
For sure, the usage of =delete should replace sf::NonCopyable. I see no reason to keep the workaround at this point.
GitHub
Code Concept
Twitter
Rosme on IRC/Discord

FRex

  • Hero Member
  • *****
  • Posts: 1839
    • View Profile
    • My GitHub Page
    • Email
Re: sf::NonCopyable and =delete
« Reply #2 on: April 04, 2017, 05:20:02 pm »
No, it's a step back. IMO.

The entire purpose of sf::NonCopyable is readability (IMO) and removing the silly trick (as explained in its hpp and I admit it's clever but still silly and a hack) from the bodies of classes that inherit from it to make the intent clear. It's not like it was impossible to make class non copyable without a base class like that, it'd just look confusing.

It's a very nice self documenting piece of code, anyone who knows this idiom (from Boost, here or wherever) and most people who don't will get it instantly - this class is not copyable, no need to look into the body AT ALL for these = delete lines. Putting the two = delete lines at the top is stupid since they are not important to the actual purpose of the class at all (just a hint about it being not copyable due to what it holds) and putting them deeper (as in binary's commit now for example) hides them so you can't tell at a glance if a class is non copyable.

It's also a change in the API, I was sometimes using this class to make my class non copyable. It's just annoying if it's gone... It's not even a serious problem, since it'd probably break code in just 3-10 places, but still annoying, all for no gain. It's creating MORE code. It's actually slightly (the added =delete) more code than it'd take to put the old trick into the bodies of classes directly. And it's even more characters and same number of lines as it is now with the base class (2 lines each, the current ones are shorter though).

Change the old trick to =delete in sf::NonCopyable itself to make errors and intent clearer (intent is already clear in the comments of NonCopyable.hpp anyway), that is VERY good, but that should be it.
ZipSavings, script to count rar/7z/zip savings: https://goo.gl/vvBj5M
LuaConsole: https://goo.gl/X4kRUk
FoxRaycaster: https://goo.gl/27nVS8
Small Games - Heart, Routing and Snek: https://goo.gl/15ZGWE https://goo.gl/k5gwWD https://goo.gl/4nKPnT
Botes - a notes app in Pascal: https://goo.gl/bzTqsi

Rosme

  • Full Member
  • ***
  • Posts: 150
  • Proud member of the shoe club
    • View Profile
    • Code-Concept
Re: sf::NonCopyable and =delete
« Reply #3 on: April 04, 2017, 06:02:10 pm »
The breaking code argument is invalid to me. Considering the amount of changes that might occur, you just can't expect your code to still build after anyway.
GitHub
Code Concept
Twitter
Rosme on IRC/Discord

Hiura

  • Moderator
  • Hero Member
  • *****
  • Posts: 4328
    • View Profile
    • Email
Re: sf::NonCopyable and =delete
« Reply #4 on: April 04, 2017, 06:24:17 pm »
While I agree that breaking code is not an issue for V3, I side with FRex. I think it really helps readability and improve the "auto-documentation" style of the code while being really concise.
SFML / OS X developer

binary1248

  • Moderator
  • Hero Member
  • *****
  • Posts: 1378
  • I am awesome.
    • View Profile
    • The server that really shouldn't be running
Re: sf::NonCopyable and =delete
« Reply #5 on: April 04, 2017, 06:37:19 pm »
People who embrace modern C++ will have to confront non-copyable types (like std::unique_ptr) at some point. We all know they won't change the standard to require deriving from an object called std::non_copyable any time soon. For me, code isn't made simpler or more intuitive just because humans can read it better. Intuition has in part to do with recognizing patterns that have been repeated from the past and applying them to different problems.

Think of it this way: For some complete beginner who starts learning C++ and reads e.g. The C++ Programming Language, they will encounter this thing called std::unique_ptr (or std::thread or many other non-copyable things) at some point. The last thing the author will probably say is "Yeah... you can't copy that... and it would have been nicer if those classes derived from a class called non_copyable, but that's what you get from design by committee." Instead, the author will probably explain that there is always a reason why copy constructor and copy assignment are explicitly marked as deleted. This is the only definition one has to know when using pure C++. Re-inventing the wheel because we disagree with the decisions of the ISO committee is just making the public interface more confusing than it has to be, especially for people who heed our advice and come to SFML with a firm understanding of standard modern C++.

Also, the fact that boost (or Qt or any other "big" library) does things in certain ways, especially considering that they had no other choice when their code was initially written, shouldn't have any meaningful impact when considering how new (or at least deliberately broken because time to clean up) code should be written. When the ISO committee chose to integrate stuff from boost into the standard, they could have went down the route of taking noncopyable along as well. Instead, they went out of their way and standardized a core language feature to solve this age old problem once and for all. If people find understanding = delete syntax hard to read or understand, I can't imagine how unpleasant it must be for them to use the STL on a daily basis.

And one last point. I think it has been made pretty clear that the ISO committee is also pushing for tools developers to provide more support for static analysis and other forms of "intelligence" that can aid the developer when writing code or when building it. I don't think anybody would disagree that from the perspective of a tool, it would be much simpler to explain to the user that something is not copyable by inspecting the AST rather than having do guess based on class names or hard coded "pattern recognition" for previously required idioms. Siding with the language features means that people who write tools for the language will automatically aid our users as well.

This Stack Overflow question/answer also provides strong arguments against keeping sf::NonCopyable.
SFGUI # SFNUL # GLS # Wyrm <- Why do I waste my time on such a useless project? Because I am awesome (first meaning).

FRex

  • Hero Member
  • *****
  • Posts: 1839
    • View Profile
    • My GitHub Page
    • Email
Re: sf::NonCopyable and =delete
« Reply #6 on: April 04, 2017, 07:42:55 pm »
I'm considering only myself here and I'm quite a programmer by now I'd say, not 'just a human'. I also want best for SFML since I don't want one of the libraries I know best to become obsolete or lose market, I get enough strange looks when people see me use Pascal in 2017. Your indifference to humans is interesting considering how very clear SFML code is to read. ;D

Intuition has nothing to do with it, especially with how SFML puts long doxygen comments in its code. You yourself placed the two = delete lines after constructor and destructor, both of which have a long comment, so it's impossible to tell at a glance if class is copyable or not. With base class it's possible and it's not like these idioms are so hard you can only learn one in your lifetime and stick to it. The non copyable base class and the = delete syntax are one of the simplest features in C++(11).

I know both so at a glance/intuitively, so 'NonCopyable' just next to a class (and let's not say it can be mixed in with others, we don't have crazy inheritance in SFML so the class Name: bases line is always super clear) wins over trying to spot = delete below in the body. Some tutorials even begin explaining = delete with an example of how it was before C++11, thus teaching both 'patterns' (if you can even call them that). And I don't think I ever used this feature past my few weeks or months with C++ and SFML so it's really not important to me but I thought it only fair to point out the benefits of how it is now. I guess I just personally consider it a good pattern that is only made better with = delete and would make my own class that does it and inherit from it to mark classes non copyable.

And just because something is a core language feature doesn't mean we have to use it, there are many (placement new, virtual inheritance, protected inheritance, class method pointers, function and void pointers, deep inheritance, multiple inheritance, etc.) features we don't use for the sake of clarity, readability and so on.

I'm not even saying to not use it. Please do use it for the benefit of clearer (than a compile or link error via obscure but clever trick that has to be explained in great detail) messages but in the non copyable base, it's clearer (IMO), self-documenting (IMO), same-ish amount to type, has no potential to skip defining one of them (and let's not claim typos or copy paste accidents don't happen - they do, ie.: the X windows 'bug' where presses and releases of mouse were jumbled together) and so on. I'm against keeping the non copyable with pre-C++11 trick but I'm also against removing it completely due to self-documenting, code breaking and seeing at a glance that a class is non copyable.


The stack overflow link proves nothing, the only argument is that friends can still copy (which is easily fixed by removing the empty bodies which edited question and some answers mention) and just says it's harder to understand on first encounter both inline and as a base class named 'noncopyable' (but that's a non argument since every language feature, hack or pattern is hard to understand at first encounter and has to be learned from some source or own experimentation and both are equally simple in the end).


Quote
The breaking code argument is invalid to me. Considering the amount of changes that might occur, you just can't expect your code to still build after anyway.
I built some one year old code this week and I'm using my LuaConsole that is from like 2014 originally, I don't see why it shouldn't build, it's a simple game and tool (routing game and my simple console) but still... SFML 2.x has an API as stable as a rock, I can't recall any problems building either.

I think we even have setColor in Text still, just deprecated. :P I'm actually not sure of any breaking changes right now (so clearly there were many big ones... ::) * ). I can only imagine frustration of someone who comes up with a bug, gets told here it's fixed in master, gets master and has to make 5-10 changes in his own code due to changed/missing function/class/whatever names. SFML doesn't backport fixes to older releases AFAIK, some projects do that but I'm not saying we should because rock stable API + 'just use master' is also very fine by me.


At this point we are getting into bike shedding territory and all the pros and cons of both were mentioned so someone somehow (Laurent, Paul, the team, coin flip, ...) should quickly decide ('upgrade' or delete NonCopyable, the 'keep pre C++11 trick' shouldn't even be an option IMO).


* And the hardest part of going from 1.9 to 2.x was changing from PascalCase to camelCase... I'm hoping that 2 to 3 will be a much smoother transition.
« Last Edit: April 04, 2017, 07:46:41 pm by FRex »
ZipSavings, script to count rar/7z/zip savings: https://goo.gl/vvBj5M
LuaConsole: https://goo.gl/X4kRUk
FoxRaycaster: https://goo.gl/27nVS8
Small Games - Heart, Routing and Snek: https://goo.gl/15ZGWE https://goo.gl/k5gwWD https://goo.gl/4nKPnT
Botes - a notes app in Pascal: https://goo.gl/bzTqsi

Ruckamongus

  • Jr. Member
  • **
  • Posts: 64
    • View Profile
Re: sf::NonCopyable and =delete
« Reply #7 on: April 04, 2017, 09:17:52 pm »
When I heard about SFML my main attraction towards it was that it used C++ natively. I began with SDL but I wasn't a fan of C much. As I learned SFML and the community more, I really appreciated the attention the big names around here gave to language standards and ideology (Nexus, binary, etc.); I learned a lot from it. Furthermore, it seems the common consensus is to stick with language provided things as much as possible (getting rid of sf::Thread for std::thread for example) and adopt C++11/14 practices. As such, keeping sf::NonCopyable feels like a half-assed transition from the older language specifications. I appreciate that inheriting from sf::NonCopyable makes the intent of the class fairly obvious, but putting all of the constructors/destructor at the top of the class definition isn't that difficult and is still a very quick skim to get the full details. Finally, SFML's documentation is stellar. I rarely ever look at the source files if I wonder about something, and mentioning a class isn't copyable in the docs is sufficient I think.

binary1248

  • Moderator
  • Hero Member
  • *****
  • Posts: 1378
  • I am awesome.
    • View Profile
    • The server that really shouldn't be running
Re: sf::NonCopyable and =delete
« Reply #8 on: April 04, 2017, 10:49:08 pm »
@FRex: It seems to me your argument rests on the assumption that the user will have to end up browsing through the source to find out whether a class is non-copyable or not.

I think it's pretty clear from my examples of the STL that that should never be the case. If someone wanted to find out whether std::thread is copyable or not, they would read some reliable documentation and find what they need without even starting to inspect the class members. The fact that std::thread is non-copyable is explained in terms of C++ Concepts, not derivation from some helper class or the deletion of special member functions. This should also be the only step necessary to figure out what I can and cannot do with a class.

Writing = delete somewhere is and never was meant as something for the programmer to read and draw assumptions from. Neither is its absence. If reading through a class was really required, then people would have a really fun time reading down to the leaves of an inheritance graph just to find out something that could have been mentioned in the first paragraph of documentation. It is a safety check for the compiler to enforce, nothing more and nothing less. The presence of = delete or derivation from sf::NonCopyable should not obviate proper documentation of the class as a non-copyable class. Simply deriving from the helper class everywhere and sending people off to look at the inheritance diagram to figure out what a derived class can or cannot do is just being lazy if you ask me (deriving from something to outright disable an action is even less trivial for beginners to understand). We already show this diagram on every page of the API documentation and yet every now and then people still ask why they can't copy these objects. I'm not saying putting = delete everywhere will solve this problem. The problem here isn't how the NonCopyable concept is implemented but how it is documented. sf::NonCopyable isn't documentation for me, and neither is = delete. Arguments that sf::NonCopyable is easier to understand without explicit documentation where it makes sense is just a symptom of lacking documentation in this regard.

Also, yes... I'm not a fan of overusing C++ language features "just because we can". All those things you listed have a right to exist, and when used properly in the right situations they can help to solve certain problems quite elegantly. Using them in the wrong scenarios as well constitutes "overuse". With explicitly deleted special member functions, I think it is pretty clear there is only one scenario where they can be put to meaningful use and it is the one we are discussing here. As such, comparing this specific language feature (which if you ask me is almost impossible to misuse) with those others that can actually be misused doesn't make much sense. It's like saying I'm not allowed to use a spoon to eat because knives are also misused to kill people. ;D
SFGUI # SFNUL # GLS # Wyrm <- Why do I waste my time on such a useless project? Because I am awesome (first meaning).

Hiura

  • Moderator
  • Hero Member
  • *****
  • Posts: 4328
    • View Profile
    • Email
Re: sf::NonCopyable and =delete
« Reply #9 on: April 05, 2017, 12:19:31 am »
On static analysers: your argument actually doesn't stand for two reasons. The first, as wildly known, one should not help the compiler by writing code in a particular way. The same rule applies to analysers. The second, when processing the AST of a given C++ class, because the language is statically typed, the parent classes, if any, are known and present in the AST. Hence, it makes no difference whatsoever which technique is applied here for analysis tools.

> Re-inventing the wheel because we disagree with the decisions of the ISO committee is just making the public interface more confusing than it has to be

Is it really reinventing the wheel? To me it looks more like a way to be more expressive, with a lighter cognitive load. Let's not forget that we are not only talking about copy operations, but also move operations.

Take this example:

(click to show/hide)



In the first case, i.e. without NonCopyable, one has to add a lot of boilerplate to the Resource class. In 5 lines, the class name is repeated 9 times. Reading this *is* expensive. And if we have a second resource class we need to do it all over again.

In the second case, the boilerplate is moved to a confined place and not repeated across the code base. The fact that the resource cannot be copied is clear right from the first line of the class if you take the time to understand why the word non-copyable means -- i.e. it's not even about C++ but about plain English.

You might argue, rightly, that one should not need to look at the source code itself but only at the documentation. But the generated documentation is very clear too: it appears at the top of the page with the composition graph and again in the list of functions. Plus, as you said, in any case we need to properly document how the class is intended to be used. Bottomline, it's a win-win.

<see attached screenshot>

Invoking that *some* users ask why they can't copy some objects is not that relevant: this part of the developer population is obviously not used to read the documentation/compiler errors, or simply are beginners. They'll ask the same questions about = delete. As you said, we have to properly document this, but with NonCopyable we get for free the = delete in the inherited functions list.

Arguing that the C++ committee didn't include std::non_copyable implies it is not something we need/want is also an irrelevant argument IMO. It's clear that the committee hasn't always taken the right decisions. But most importantly, they don't need to solve every issues and don't want to impose too many design choices on the users.
SFML / OS X developer

binary1248

  • Moderator
  • Hero Member
  • *****
  • Posts: 1378
  • I am awesome.
    • View Profile
    • The server that really shouldn't be running
Re: sf::NonCopyable and =delete
« Reply #10 on: April 05, 2017, 01:56:07 am »
The first, as wildly known, one should not help the compiler by writing code in a particular way. The same rule applies to analysers.
This only applies to the context of optimizations. When it comes to the matter of correctness, whether we specify a function with override or mark a function as deleted, it is not a matter of who does it better, but of what we want to tell the compiler. We speak to the compiler in terms of syntax and the C++ language, not English or inheritance relations. Moving functions to have private accessibility (and importantly this is not the same as visibility) so they can't be accessed outside the class is not equivalent to marking them as deleted thus preventing the compiler from generating them at all. We aren't "helping" the compiler here. In either case we are telling it exactly what it should do, the "what" is what differs.

The second, when processing the AST of a given C++ class, because the language is statically typed, the parent classes, if any, are known and present in the AST. Hence, it makes no difference whatsoever which technique is applied here for analysis tools.
This is assuming that analysers will exhaustively scan the AST to make sure it didn't miss anything. Just like optimizations, analysis isn't governed by any specification. Implementers are free to do what they think serves their userbase the best. If they need to employ heuristics that only cover 99% of all cases to keep the complexity of the task in the realm of the feasible, they will also do so. I'm not saying that there can't be tools that will understand the inheritance relation as well, we just have a higher chance of getting tools to "do the right thing" if we keep the AST as simple as possible. Inheriting from sf::NonCopyable multiple times as is the case with some SFML classes only contributes to unnecessary complexity that might turn out to be a disadvantage in some cases.

Is it really reinventing the wheel? To me it looks more like a way to be more expressive, with a lighter cognitive load. Let's not forget that we are not only talking about copy operations, but also move operations.
The factors that determine the true cognitive load differ from person to person in part based on past experience as well. What I can say is that for a hypothetical person, who has just learned C++ post-11 and never seen this non-copyable base class pattern before, it will definitely incur a higher cognitive load because they unarguably have to learn something new in addition to what they have learned as part of the language itself. sf::NonCopyable is optimizing expressiveness in favour of "experienced" people who recognize the pattern from pre-11 C++, whereas = delete is optimizing expressiveness in favour of "novice" people who are only familiar with the standard post-11 C++ idioms employed in the STL. As I've stated before, one way or the other, C++ developers whether they are new or experienced will have to deal with the way the STL is specified. Forcing them to learn something else in addition to that because we follow other philosophies doesn't make SFML seem like a "native" C++ library for people who expected adherence to principles they know from elsewhere (such as the STL).

As for move operations, do you propose we introduce a sf::NonMoveable as well? Because inheriting from both classes to make something non-copyable and non-movable makes it unreadable if you ask me. Yes... one day Concepts will solve all these problems (hopefully), but until then, we should really just stick to the bone the committee has thrown us to bridge the gap. As already stated, the deletion of these member functions serves no other purpose than this, so it is kind of obvious this is the idiomatic solution to solve this problem according to the committee.

In the first case, i.e. without NonCopyable, one has to add a lot of boilerplate to the Resource class. In 5 lines, the class name is repeated 9 times. Reading this *is* expensive. And if we have a second resource class we need to do it all over again.
I generally don't tend to measure the cost of reading code by the repetitiveness of certain tokens. If instead of the class name the signatures would contain 9 different tokens, it would be just the same for me. Considering some of SFML's function signatures can get quite long and number of member functions can grow quite large, I don't think it makes sense to judge the way something is implemented based among other things on lines of code or the length of those lines. When I read these lines as a human, my brain recognizes patterns, beginning with the = delete at the end. When I see 4 of them, I can stop reading because I know the class is neither copyable nor moveable. If I see only 2 of them, I look for double ampersands. If I don't find any, I know the class is non-copyable but movable. Maybe it's just the way I've learned to read code, but surprisingly, I've noticed that when I try to understand code, it doesn't boil down to how much is written, but often how it is written. In this aspect, my brain prefers longer, more regular patterns over shorter "non-patterns" regardless of spoken language.

In the second case, the boilerplate is moved to a confined place and not repeated across the code base. The fact that the resource cannot be copied is clear right from the first line of the class if you take the time to understand why the word non-copyable means -- i.e. it's not even about C++ but about plain English.
We would be putting all our eggs in one basket by requiring it to be evident from the first line of the class whether a class is copyable or not. The class won't be copyable even when not inheriting from sf::NonCopyable as soon as one of its members is non-copyable, either through sf::NonCopyable or some STL class that ultimately uses = delete. All I'm saying is that the argument that it is obvious with sf::NonCopyable cannot be made for the general case. There are cases where it will be just as "unobvious" as = delete, and like I just said, if you have classes with both sf::NonCopyable and non-copyable STL classes as members, the compiler will be the last one to care about not confusing the developer with multiple differing error messages.

You might argue, rightly, that one should not need to look at the source code itself but only at the documentation. But the generated documentation is very clear too: it appears at the top of the page with the composition graph and again in the list of functions. Plus, as you said, in any case we need to properly document how the class is intended to be used. Bottomline, it's a win-win.
This is just a matter of where doxygen places the inheritance graph. I must admit, the first time I saw doxygen documentation I also asked myself why the diagram was placed before the member list let alone the synopsis. The inheritance relation for me is part implementation part interface specification. In any case, it really shouldn't take precedence over a thorough description of what the class is actually supposed to be good for. If while reading the description one realizes that the class isn't really the thing they need, they wouldn't have to bother understanding what it inherits from. The graph itself is incomplete information and as such for me doesn't really belong where it currently does, but after so long I guess I just got used to it (if I even bother to look at it for more than 1 second).

Invoking that *some* users ask why they can't copy some objects is not that relevant: this part of the developer population is obviously not used to read the documentation/compiler errors, or simply are beginners. They'll ask the same questions about = delete.
Maybe they might, but we can't blame them for coming to us without doing some research on their own when we don't even make sure they get error messages they can easily search for. I didn't check, but I have a feeling you will get more hits regarding "is explicitly marked as deleted" as opposed to "sf::NonCopyable::operator= is private in this context".

As you said, we have to properly document this, but with NonCopyable we get for free the = delete in the inherited functions list.
Again, I haven't checked, but what I noticed is that inherited constructors or assignment operators aren't displayed in the derived class. Whether or not deleted functions are even listed is another question altogether.

Arguing that the C++ committee didn't include std::non_copyable implies it is not something we need/want is also an irrelevant argument IMO. It's clear that the committee hasn't always taken the right decisions. But most importantly, they don't need to solve every issues and don't want to impose too many design choices on the users.
But in this case, this is an issue that they spent their time solving, with a more or less concrete solution. The inclusion of smart pointers into the standard also doesn't prevent us from still using the boost versions either. It's just a matter of convenience. Yes... I can imagine there were people on the committee that might have been opposed to this idea, but so is the nature of design by committee. They might not always make the right decisions, but when decisions are made, they become part of the language standard, the standard everybody has to adhere to especially a library written in C++. We can resist it if we want, but in that case we should refrain from sending people off to learn modern C++ only to tell them after they are done that we don't find everything we told them to learn that great either.
SFGUI # SFNUL # GLS # Wyrm <- Why do I waste my time on such a useless project? Because I am awesome (first meaning).

Hiura

  • Moderator
  • Hero Member
  • *****
  • Posts: 4328
    • View Profile
    • Email
Re: sf::NonCopyable and =delete
« Reply #11 on: April 05, 2017, 03:04:42 pm »
Moving functions to have private accessibility (and importantly this is not the same as visibility) so they can't be accessed outside the class is not equivalent to marking them as deleted thus preventing the compiler from generating them at all. We aren't "helping" the compiler here. In either case we are telling it exactly what it should do, the "what" is what differs.
I think there's a slight misunderstanding: I'm not arguing against = delete, I'm arguing we should keep NonCopyable. Using = delete as an implementation details in NonCopyable is obviously the right decision.

There's helping the optimiser/compiler/analyser/whatever and there's leveraging the type system to write safer programs. The two are really orthogonal. Using `override`, `private`, `= delete`, inheritance, ... is all about the type system. Which technique you use to tackle a given problem should ideally not be determined by the optimiser/compiler/analyser/whatever needs.

Quote
This is assuming that analysers will exhaustively scan the AST to make sure it didn't miss anything.

I really hope the analysers you're using do that! ;) If they don't even extract the full AST (or don't do at least type checking), they'll not be giving any useful hint in many many cases. In this case, specifically, the tools needs to know the full class hierarchy to determine whether a type is copyable/movable/..., so it wouldn't make sense to not inspect the parent classes.

Quote
The factors that determine the true cognitive load differ from person to person in part based on past experience as well.

Sure, for little things maybe, but if you reduce the code size you're helping everyone. More importantly, the general rule should be, in my view, that we should use names to convey purpose instead of using language features to look cool. Now, everyone has their own definition of "cool", but here it really boils down to "one identifier = one concept". Why change that?


Quote
As for move operations, do you propose we introduce a sf::NonMoveable as well?
At the present time, I see no reason to do that. Which class should not be moveable?

Quote
I generally don't tend to measure the cost of reading code by the repetitiveness of certain tokens. If instead of the class name the signatures would contain 9 different tokens, it would be just the same for me.
I'm happy for you that you can deal with this very efficiently, but I'm not convince it's the case for the majority of human beings. I mean, if it weren't the case project such as https://github.com/tonsky/FiraCode wouldn't be needed. It's also what `auto` is (partially) about: reducing to what really matters. You can also find many improvements on the syntax, especially with functional languages, where a lighter syntax -- and therefore a lighter cognitive load -- allows to improve expressiveness.


Quote
We would be putting all our eggs in one basket by requiring it to be evident from the first line of the class whether a class is copyable or not.
If we were to assume one doesn't need to read the class documentation, then yes. But I think we agree that this assumption is incorrect. So I see no issue here.

Quote
I have a feeling you will get more hits regarding "is explicitly marked as deleted" as opposed to "sf::NonCopyable::operator= is private in this context".

More hits, but the quality of those might not be better. I just did the experiment and the results were much better with NonCopyable. Of course you might get different results depending on your search engines and cookies... but if you take the example I gave earlier, you actually get both hints with clangs (see attached image). So, again, to me it's a win-win.

Quote
Again, I haven't checked, but what I noticed is that inherited constructors or assignment operators aren't displayed in the derived class. Whether or not deleted functions are even listed is another question altogether.
Not sure if it's relevant, but they do get listed as well. See the attached example where I've added some dummy ctor/op=.

Quote
We can resist it if we want, but in that case we should refrain from sending people off to learn modern C++ only to tell them after they are done that we don't find everything we told them to learn that great either.
Let me disagree. ;-) People should learn what alternatives exist and then choose the one that fits best their challenge. In a way, that's what we're doing here: pondering whether it's better to do X or Y to solve P. A daily task for (C++) programmers anyway; e.g. you can use many container to solve P but some are better at A than B. Designing class abides to the same law, I'm sure you'll agree. So generally speaking, sending people to read on modern C++ is always a good thing, but they should not leave their critical opinion behind.


Regarding the STL, one major difference with SFML is that many std classes needs custom move operations. I believe we don't for most classes if not all. With the STL, it wouldn't make sense to both inherit from non_copyable and defined the custom operators. This probably explains why they didn't introduce non_copyable too: no need for it.
SFML / OS X developer

korczurekk

  • Full Member
  • ***
  • Posts: 150
    • View Profile
    • Email
Re: sf::NonCopyable and =delete
« Reply #12 on: April 06, 2017, 02:56:21 pm »
While I personally agree with @Hiura, I think that this specific dilemma should be:
  • Moved to separate topic. Too much OT for this sub–forum, don't you think?
  • Decided through query as it is almost completely design choice and neither of solutions can be proven to be better than other one.

eXpl0it3r

  • Moderator
  • Hero Member
  • *****
  • Posts: 9221
    • View Profile
    • development blog
    • Email
Re: sf::NonCopyable and =delete
« Reply #13 on: April 06, 2017, 03:04:25 pm »


I think that this specific dilemma should be:
  • Moved to separate topic. Too much OT for this sub–forum, don't you think?
  • Decided through query as it is almost completely design choice and neither of solutions can be proven to be better than other one.
I'll move the discussion to a dedicated thread when I get home, but it's exactly these kind of discussions I want to see in this sub-forum!
Not sure what you meant by query? There's no point in having a private discussion about it, however we'll have to find a way to come to a decision. But before jumping to anything, I'd like to hear the opinions of other team members.
Official FAQ: https://www.sfml-dev.org/faq.php
Nightly Builds: https://www.nightlybuilds.ch/
——————————————————————
Dev Blog: https://dev.my-gate.net/
Thor: http://www.bromeon.ch/libraries/thor/

korczurekk

  • Full Member
  • ***
  • Posts: 150
    • View Profile
    • Email
Re: sf::NonCopyable and =delete
« Reply #14 on: April 06, 2017, 03:27:35 pm »
I'll move the discussion to a dedicated thread when I get home, but it's exactly these kind of discussions I want to see in this sub-forum!
I meant that there should be no OT in this sub-forum, i.e. each thing should be discussed in proper thread (:P) while this one certainly doesn't fit 'C++ standards' name.

Not sure what you meant by query? There's no point in having a private discussion about it, however we'll have to find a way to come to a decision. But before jumping to anything, I'd like to hear the opinions of other team members.
By query I referred to questionnaire/form in which qualified users would be allowed to vote for proffered design.