[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

[Konrad Kleine via Phabricator via lldb-commits]

kkleine created this revision.
Herald added subscribers: lldb-commits, kadircet, arphaman.
Herald added a project: LLDB.

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 out 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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61440

Files:
  lldb/include/lldb/Core/Architecture.h
  lldb/include/lldb/Core/StreamBuffer.h
  lldb/include/lldb/Core/ValueObjectVariable.h
  lldb/include/lldb/Target/DynamicLoader.h
  lldb/include/lldb/Target/StackFrameRecognizer.h
  lldb/include/lldb/Target/StructuredDataPlugin.h
  lldb/include/lldb/Utility/Baton.h
  lldb/include/lldb/Utility/DataBufferLLVM.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/API/SBBreakpointOptionCommon.h
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptExpressionOpts.h
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.h
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/source/Symbol/PostfixExpression.cpp
  lldb/source/Target/StructuredDataPlugin.cpp
  lldb/tools/intel-features/intel-mpx/cli-wrapper-mpxtable.cpp
  lldb/unittests/Editline/EditlineTest.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
  lldb/unittests/Symbol/TestClangASTContext.cpp
  lldb/unittests/Target/ProcessInstanceInfoTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61440.197767.patch
Type: text/x-patch
Size: 24589 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190502/26fe1b6f/attachment-0001.bin>