-
Notifications
You must be signed in to change notification settings - Fork 772
Should the library use override, instead of virtual, in derived classes? #751
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
Comments
I agree both that this is a good idea (the library synopses should use idiomatic modern C++), and editorial. @mclow, do you want to run this past the lib- reflector before we go ahead with it? We can easily back it out if there turns out to be reasonable opposition.
Yes, but only for those cases, so if you find any when making this change, you can just skip those ones :) |
I'm OK with this as well. While I would think that running it by the -lib reflector would normally be prudent, I think Richard's two points of (a) editorial, and (b) easily reverted are enough. I think a "we made this change" to the -lib reflector (i.e, post-commit review) will be fine. |
This patch makes consistent use of the 'override' contextual keyword for every member function (other than destructors) that overrides a virtual function in one of its base classes. In general, this came down to one of three cases: 1) the 'what' function for an exception class 2) the allocation functions of a memory resource 3) the implementation methods of a stream buffer. As far as I can tell, all other virtual function declarations in the standard library are intended as customization points for users, and are not overridden in the standard itself. Fixes cplusplus#751
I believe the use of the override (contextual) keyword, rather than virtual, when declaring an overriding function in a class definition, is purely a matter of style, not semantics. As such, it should be addressed (if at all) as an editorial issue.
I believe users of more recent dialects of C++ would expect this syntax, and we should actively make such a change - and will prepare a pull request asap if the editors show interest.
There are two main scenarios:
deriving from a regular class, such as bad_exception::what overriding exception::what.
such cases are relatively straight forward, and should pose no concerns
overriding a virtual function in a dependent base class, such as
basic_filebuf<CHAR, TRAITS>::overflow overriding basic_streambuf<CHAR, TRAITS>::overflow.
The concern here is that a user specialization might not provide the virtual function, so adding override becomes a diagnosable error, and such a change might merit LWG approval via the issues process. However, the user is constrained that their specialization must implement the interface of the primary template, so they have always been required to provide that virtual function with exactly that signature, so I claim this can still be reasonably handled editorially.
There are no cases in the library of a putative case 3, where a type derives directly from a template parameter, where it would be entirely unknown what virtual function the parameterizing class would have. If there are such cases and I have missed them, use of override would clearly be non-editorial.
The text was updated successfully, but these errors were encountered: