mirror of
https://github.com/isocpp/CppCoreGuidelines.git
synced 2024-03-22 13:30:58 +08:00
Closes #1446
This PR affirms that all virtual functions, *including destructors*, should be declared exactly one of `virtual`, `override`, or `final`, and takesa pass through the document to make the examples and guidance consistent with that. Of course a virtual destructor is a virtual function: It behaves polymorphically, and it has a vtable entry that can be overwritten == overridden in a derived class exactly the same as any other derived virtual override. See also [class.virtual]/7: "Even though destructors are not inherited, a destructor in a derived class overrides a base class destructor declared virtual; see [class.dtor] and [class.free]." However, the following exception text currently appears in C.128: > If a base class destructor is declared `virtual`, one should avoid declaring derived class destructors `virtual` or `override`. Some code base and tools might insist on `override` for destructors, but that is not the recommendation of these guidelines. ... but this exception is (a) not well-founded, and (b) inconsistent with the Guidelines' practice in other examples and with the rationale a few lines earlier for C.128 itself. Re (a): - The exception is overly broad: The rationale given for this exception is entirely against marking destructors `override` (not `virtual`). So clearly the exception to write neither keyword is too broad: At most, the exception should be to write `virtual` rather than `override`. - Explicit `virtual` is primarily for class users, not class authors: The arguments given in #721 favoring this exception are from the viewpoint of the implementation of the function (even then, the arguments are debatable and debated). But `virtual`, `override`, and `final` are primarily for the far larger audience of *class users and call sites* of the function, for whom of course we should document each declared function that is polymorphic, *especially* the destructor -- this tells calling code whether the function is safe to call through a (smart or built-in) pointer or reference to base, which will nearly always be the case for such types. We should not make the reader of the code go way to look in the base classes to figure out whether a function declared in this class is virtual or not -- the reason this Item exists is primarily to avoid that implicit virtual antipattern via convention and automated enforcement. For class users, all virtual functions including destructors are equally polymorphic. Re (b): The Guidelines already don't follow this. For instance, two Items later (in C.130) we have this example that correctly uses `override`: ~~~ virtual ~D() override; ~~~ ... though per C.128 it should not also specify `virtual` (also fixed in this PR). Finally, the exception also contradicts the rationale given earlier in the same Item.
This commit is contained in:
parent
4b414458cf
commit
fcb2960793
|
@ -1,6 +1,6 @@
|
||||||
# <a name="main"></a>C++ Core Guidelines
|
# <a name="main"></a>C++ Core Guidelines
|
||||||
|
|
||||||
May 2, 2019
|
June 16, 2019
|
||||||
|
|
||||||
|
|
||||||
Editors:
|
Editors:
|
||||||
|
@ -6305,6 +6305,7 @@ Worse, a direct or indirect call to an unimplemented pure virtual function from
|
||||||
virtual void f() = 0; // not implemented
|
virtual void f() = 0; // not implemented
|
||||||
virtual void g(); // implemented with Base version
|
virtual void g(); // implemented with Base version
|
||||||
virtual void h(); // implemented with Base version
|
virtual void h(); // implemented with Base version
|
||||||
|
virtual ~Base(); // implemented with Base version
|
||||||
};
|
};
|
||||||
|
|
||||||
class Derived : public Base {
|
class Derived : public Base {
|
||||||
|
@ -6977,8 +6978,6 @@ It's simple and clear:
|
||||||
* `override` means exactly and only "this is a non-final overrider."
|
* `override` means exactly and only "this is a non-final overrider."
|
||||||
* `final` means exactly and only "this is a final overrider."
|
* `final` means exactly and only "this is a final overrider."
|
||||||
|
|
||||||
If a base class destructor is declared `virtual`, one should avoid declaring derived class destructors `virtual` or `override`. Some code base and tools might insist on `override` for destructors, but that is not the recommendation of these guidelines.
|
|
||||||
|
|
||||||
##### Example, bad
|
##### Example, bad
|
||||||
|
|
||||||
struct B {
|
struct B {
|
||||||
|
@ -7260,7 +7259,7 @@ Copying a polymorphic class is discouraged due to the slicing problem, see [C.67
|
||||||
class B {
|
class B {
|
||||||
public:
|
public:
|
||||||
virtual owner<B*> clone() = 0;
|
virtual owner<B*> clone() = 0;
|
||||||
virtual ~B() = 0;
|
virtual ~B() = default;
|
||||||
|
|
||||||
B(const B&) = delete;
|
B(const B&) = delete;
|
||||||
B& operator=(const B&) = delete;
|
B& operator=(const B&) = delete;
|
||||||
|
@ -7269,7 +7268,7 @@ Copying a polymorphic class is discouraged due to the slicing problem, see [C.67
|
||||||
class D : public B {
|
class D : public B {
|
||||||
public:
|
public:
|
||||||
owner<D*> clone() override;
|
owner<D*> clone() override;
|
||||||
virtual ~D() override;
|
~D() override;
|
||||||
};
|
};
|
||||||
|
|
||||||
Generally, it is recommended to use smart pointers to represent ownership (see [R.20](#Rr-owner)). However, because of language rules, the covariant return type cannot be a smart pointer: `D::clone` can't return a `unique_ptr<D>` while `B::clone` returns `unique_ptr<B>`. Therefore, you either need to consistently return `unique_ptr<B>` in all overrides, or use `owner<>` utility from the [Guidelines Support Library](#SS-views).
|
Generally, it is recommended to use smart pointers to represent ownership (see [R.20](#Rr-owner)). However, because of language rules, the covariant return type cannot be a smart pointer: `D::clone` can't return a `unique_ptr<D>` while `B::clone` returns `unique_ptr<B>`. Therefore, you either need to consistently return `unique_ptr<B>` in all overrides, or use `owner<>` utility from the [Guidelines Support Library](#SS-views).
|
||||||
|
@ -7545,6 +7544,7 @@ Without a using declaration, member functions in the derived class hide the enti
|
||||||
public:
|
public:
|
||||||
virtual int f(int i) { std::cout << "f(int): "; return i; }
|
virtual int f(int i) { std::cout << "f(int): "; return i; }
|
||||||
virtual double f(double d) { std::cout << "f(double): "; return d; }
|
virtual double f(double d) { std::cout << "f(double): "; return d; }
|
||||||
|
virtual ~B() = default;
|
||||||
};
|
};
|
||||||
class D: public B {
|
class D: public B {
|
||||||
public:
|
public:
|
||||||
|
@ -7632,6 +7632,7 @@ That can cause confusion: An overrider does not inherit default arguments.
|
||||||
class Base {
|
class Base {
|
||||||
public:
|
public:
|
||||||
virtual int multiply(int value, int factor = 2) = 0;
|
virtual int multiply(int value, int factor = 2) = 0;
|
||||||
|
virtual ~Base() = default;
|
||||||
};
|
};
|
||||||
|
|
||||||
class Derived : public Base {
|
class Derived : public Base {
|
||||||
|
@ -7659,7 +7660,7 @@ If you have a class with a virtual function, you don't (in general) know which c
|
||||||
|
|
||||||
##### Example
|
##### Example
|
||||||
|
|
||||||
struct B { int a; virtual int f(); };
|
struct B { int a; virtual int f(); virtual ~B() = default };
|
||||||
struct D : B { int b; int f() override; };
|
struct D : B { int b; int f() override; };
|
||||||
|
|
||||||
void use(B b)
|
void use(B b)
|
||||||
|
@ -7702,6 +7703,7 @@ Flag all slicing.
|
||||||
struct B { // an interface
|
struct B { // an interface
|
||||||
virtual void f();
|
virtual void f();
|
||||||
virtual void g();
|
virtual void g();
|
||||||
|
virtual ~B();
|
||||||
};
|
};
|
||||||
|
|
||||||
struct D : B { // a wider interface
|
struct D : B { // a wider interface
|
||||||
|
@ -21507,6 +21509,8 @@ Here is an example of the last option:
|
||||||
p->post_initialize();
|
p->post_initialize();
|
||||||
return p;
|
return p;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ...
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue
Block a user