HomePhabricator

C.128 override, virtual keyword handling

Description

C.128 override, virtual keyword handling

Summary:
According to [C128] "Virtual functions should specify exactly one
of virtual, override, or final", I've added override where a
virtual function is overriden but the explicit override keyword
was missing. Whenever both virtual and override were specified,
I removed virtual. As C.128 puts it:

[...] writing more than one of these three is both redundant and
a potential source of errors.

I anticipate a discussion about whether or not to add override to
destructors but I went for it because of an example in [ISOCPP1000].
Let me repeat the comment for you here:

Consider this code:

struct Base {
  virtual ~Base(){}
};

struct SubClass : Base {
  ~SubClass() {
    std::cout << "It works!\n";
  }
};

int main() {
  std::unique_ptr<Base> ptr = std::make_unique<SubClass>();
}

If for some odd reason somebody removes the virtual keyword from the
Base struct, the code will no longer print It works!. So adding
override to destructors actively protects us from accidentally
breaking our code at runtime.

[C128]: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c128-virtual-functions-should-specify-exactly-one-of-virtual-override-or-final
[ISOCPP1000]: https://github.com/isocpp/CppCoreGuidelines/issues/1000#issuecomment-476951555

Reviewers: teemperor, JDevlieghere, davide, shafik

Reviewed By: teemperor

Subscribers: kwk, arphaman, kadircet, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D61440

Details

Committed
teemperorMay 3 2019, 3:03 AM
Reviewer
teemperor
Differential Revision
D61440: C.128 override, virtual keyword handling
Parents
rL359867: Split TestVLA into two and XFAIL one part
Branches
Unknown
Tags
Unknown