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]: isocpp/CppCoreGuidelines#1000 (comment)

Reviewers: teemperor, JDevlieghere, davide, shafik

Reviewed By: teemperor

Subscribers: kwk, arphaman, kadircet, lldb-commits

Tags: #lldb

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

llvm-svn: 359868

lldb/include/lldb/Core/Architecture.h

@@ -16,7 +16,7 @@ namespace lldb_private {
 class Architecture : public PluginInterface {
 public:
   Architecture() = default;
-  virtual ~Architecture() = default;
+  ~Architecture() override = default;
 
   /// This is currently intended to handle cases where a
   /// program stops at an instruction that won't get executed and it

lldb/include/lldb/Core/StreamBuffer.h

@@ -23,9 +23,9 @@ template <unsigned N> class StreamBuffer : public Stream {
   StreamBuffer(uint32_t flags, uint32_t addr_size, lldb::ByteOrder byte_order)
       : Stream(flags, addr_size, byte_order), m_packet() {}
 
-  virtual ~StreamBuffer() {}
+  ~StreamBuffer() override {}
 
-  virtual void Flush() {
+  void Flush() override {
     // Nothing to do when flushing a buffer based stream...
   }
 
@@ -42,7 +42,7 @@ template <unsigned N> class StreamBuffer : public Stream {
 protected:
   llvm::SmallVector<char, N> m_packet;
 
-  virtual size_t WriteImpl(const void *s, size_t length) {
+  size_t WriteImpl(const void *s, size_t length) override {
     if (s && length)
       m_packet.append((const char *)s, ((const char *)s) + length);
     return length;

lldb/include/lldb/Core/ValueObjectVariable.h

@@ -74,7 +74,7 @@ class ValueObjectVariable : public ValueObject {
 
   bool SetData(DataExtractor &data, Status &error) override;
 
-  virtual lldb::VariableSP GetVariable() override { return m_variable_sp; }
+  lldb::VariableSP GetVariable() override { return m_variable_sp; }
 
 protected:
   bool UpdateValue() override;

lldb/include/lldb/Target/DynamicLoader.h

@@ -84,7 +84,7 @@ class DynamicLoader : public PluginInterface {
   ///
   /// The destructor is virtual since this class is designed to be inherited
   /// from by the plug-in instance.
-  virtual ~DynamicLoader() override;
+  ~DynamicLoader() override;
 
   /// Called after attaching a process.
   ///

lldb/include/lldb/Target/StackFrameRecognizer.h

@@ -73,7 +73,7 @@ class ScriptedStackFrameRecognizer : public StackFrameRecognizer {
 public:
   ScriptedStackFrameRecognizer(lldb_private::ScriptInterpreter *interpreter,
                                const char *pclass);
-  ~ScriptedStackFrameRecognizer() {}
+  ~ScriptedStackFrameRecognizer() override {}
 
   std::string GetName() override {
     return GetPythonClassName();

lldb/include/lldb/Target/StructuredDataPlugin.h

@@ -41,7 +41,7 @@ class StructuredDataPlugin
     : public PluginInterface,
       public std::enable_shared_from_this<StructuredDataPlugin> {
 public:
-  virtual ~StructuredDataPlugin();
+  ~StructuredDataPlugin() override;
 
   lldb::ProcessSP GetProcess() const;
 

lldb/include/lldb/Utility/Baton.h

@@ -44,7 +44,7 @@ class Baton {
 class UntypedBaton : public Baton {
 public:
   UntypedBaton(void *Data) : m_data(Data) {}
-  virtual ~UntypedBaton() {
+  ~UntypedBaton() override {
     // The default destructor for an untyped baton does NOT attempt to clean up
     // anything in m_data.
   }
@@ -63,8 +63,7 @@ template <typename T> class TypedBaton : public Baton {
   const T *getItem() const { return Item.get(); }
 
   void *data() override { return Item.get(); }
-  virtual void GetDescription(Stream *s,
-                              lldb::DescriptionLevel level) const override {}
+  void GetDescription(Stream *s, lldb::DescriptionLevel level) const override {}
 
 protected:
   std::unique_ptr<T> Item;

lldb/include/lldb/Utility/DataBufferLLVM.h

@@ -25,7 +25,7 @@ namespace lldb_private {
 class FileSystem;
 class DataBufferLLVM : public DataBuffer {
 public:
-  ~DataBufferLLVM();
+  ~DataBufferLLVM() override;
 
   uint8_t *GetBytes() override;
   const uint8_t *GetBytes() const override;

lldb/include/lldb/Utility/StringExtractorGDBRemote.h

@@ -34,7 +34,7 @@ class StringExtractorGDBRemote : public StringExtractor {
   StringExtractorGDBRemote(const StringExtractorGDBRemote &rhs)
       : StringExtractor(rhs), m_validator(rhs.m_validator) {}
 
-  virtual ~StringExtractorGDBRemote() {}
+  ~StringExtractorGDBRemote() override {}
 
   bool ValidateResponse() const;
 

lldb/source/API/SBBreakpointOptionCommon.h

@@ -24,7 +24,7 @@ class SBBreakpointCallbackBaton : public lldb_private::TypedBaton<CallbackData>
   SBBreakpointCallbackBaton(SBBreakpointHitCallback callback,
                             void *baton);
 
-  ~SBBreakpointCallbackBaton();
+  ~SBBreakpointCallbackBaton() override;
 
   static bool PrivateBreakpointHitCallback(void *baton,
                                            lldb_private::StoppointCallbackContext *ctx,

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp

@@ -94,7 +94,7 @@ class DynamicLoaderDarwinKernelProperties : public Properties {
     m_collection_sp->Initialize(g_properties);
   }
 
-  virtual ~DynamicLoaderDarwinKernelProperties() {}
+  ~DynamicLoaderDarwinKernelProperties() override {}
 
   bool GetLoadKexts() const {
     const uint32_t idx = ePropertyLoadKexts;

lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h

@@ -28,7 +28,7 @@ class DynamicLoaderDarwin : public lldb_private::DynamicLoader {
 public:
   DynamicLoaderDarwin(lldb_private::Process *process);
 
-  virtual ~DynamicLoaderDarwin() override;
+  ~DynamicLoaderDarwin() override;
 
   /// Called after attaching a process.
   ///

lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h

@@ -32,7 +32,7 @@ class DynamicLoaderMacOS : public lldb_private::DynamicLoaderDarwin {
 public:
   DynamicLoaderMacOS(lldb_private::Process *process);
 
-  virtual ~DynamicLoaderMacOS() override;
+  ~DynamicLoaderMacOS() override;
 
   // Static Functions
   static void Initialize();

lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h

@@ -36,7 +36,7 @@ class DynamicLoaderMacOSXDYLD : public lldb_private::DynamicLoaderDarwin {
 public:
   DynamicLoaderMacOSXDYLD(lldb_private::Process *process);
 
-  virtual ~DynamicLoaderMacOSXDYLD() override;
+  ~DynamicLoaderMacOSXDYLD() override;
 
   // Static Functions
   static void Initialize();

lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h

@@ -33,7 +33,7 @@ class ClangDiagnostic : public Diagnostic {
                   uint32_t compiler_id)
       : Diagnostic(message, severity, eDiagnosticOriginClang, compiler_id) {}
 
-  virtual ~ClangDiagnostic() = default;
+  ~ClangDiagnostic() override = default;
 
   bool HasFixIts() const override { return !m_fixit_vec.empty(); }
 

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

@@ -153,7 +153,7 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
   }
 
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
-                        const clang::Diagnostic &Info) {
+                        const clang::Diagnostic &Info) override {
     if (m_manager) {
       llvm::SmallVector<char, 32> diag_str;
       Info.FormatDiagnostic(diag_str);
@@ -719,7 +719,7 @@ class CodeComplete : public CodeCompleteConsumer {
   }
 
   /// Deregisters and destroys this code-completion consumer.
-  virtual ~CodeComplete() {}
+  ~CodeComplete() override {}
 
   /// \name Code-completion filtering
   /// Check if the result should be filtered out.

lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

@@ -914,7 +914,7 @@ static void LoadSystemFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
 std::unique_ptr<Language::TypeScavenger> CPlusPlusLanguage::GetTypeScavenger() {
   class CPlusPlusTypeScavenger : public Language::ImageListTypeScavenger {
   public:
-    virtual CompilerType AdjustForInclusion(CompilerType &candidate) override {
+    CompilerType AdjustForInclusion(CompilerType &candidate) override {
       LanguageType lang_type(candidate.GetMinimumLanguage());
       if (!Language::LanguageIsC(lang_type) &&
           !Language::LanguageIsCPlusPlus(lang_type))

lldb/source/Plugins/Language/ObjC/NSDictionary.h

@@ -51,17 +51,17 @@ class NSDictionary_Additionals {
     class Prefix : public Matcher {
     public:
       Prefix(ConstString p);
-      virtual ~Prefix() = default;
-      virtual bool Match(ConstString class_name) override;
+      ~Prefix() override = default;
+      bool Match(ConstString class_name) override;
 
     private:
       ConstString m_prefix;
     };
     class Full : public Matcher {
     public:
       Full(ConstString n);
-      virtual ~Full() = default;
-      virtual bool Match(ConstString class_name) override;
+      ~Full() override = default;
+      bool Match(ConstString class_name) override;
 
     private:
       ConstString m_name;

lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp

@@ -994,7 +994,7 @@ std::unique_ptr<Language::TypeScavenger> ObjCLanguage::GetTypeScavenger() {
 
   class ObjCDebugInfoScavenger : public Language::ImageListTypeScavenger {
   public:
-    virtual CompilerType AdjustForInclusion(CompilerType &candidate) override {
+    CompilerType AdjustForInclusion(CompilerType &candidate) override {
       LanguageType lang_type(candidate.GetMinimumLanguage());
       if (!Language::LanguageIsObjC(lang_type))
         return CompilerType();

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

@@ -2659,7 +2659,8 @@ class ObjCExceptionRecognizedStackFrame : public RecognizedStackFrame {
 };
 
 class ObjCExceptionThrowFrameRecognizer : public StackFrameRecognizer {
-  lldb::RecognizedStackFrameSP RecognizeFrame(lldb::StackFrameSP frame) {
+  lldb::RecognizedStackFrameSP
+  RecognizeFrame(lldb::StackFrameSP frame) override {
     return lldb::RecognizedStackFrameSP(
         new ObjCExceptionRecognizedStackFrame(frame));
   };

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptExpressionOpts.h

@@ -34,7 +34,7 @@ class RenderScriptRuntimeModulePass : public llvm::ModulePass {
   RenderScriptRuntimeModulePass(const lldb_private::Process *process)
       : ModulePass(ID), m_process_ptr(process) {}
 
-  bool runOnModule(llvm::Module &module);
+  bool runOnModule(llvm::Module &module) override;
 
 private:
   const lldb_private::Process *m_process_ptr;

lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

@@ -113,7 +113,7 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile {
 
   uint32_t GetDependentModules(lldb_private::FileSpecList &files) override;
 
-  virtual lldb_private::Address GetEntryPointAddress() override;
+  lldb_private::Address GetEntryPointAddress() override;
 
   lldb_private::Address GetBaseAddress() override;
 

lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h

@@ -33,7 +33,7 @@ class PlatformRemoteGDBServer : public Platform, private UserIDResolver {
 
   PlatformRemoteGDBServer();
 
-  virtual ~PlatformRemoteGDBServer();
+  ~PlatformRemoteGDBServer() override;
 
   // lldb_private::PluginInterface functions
   ConstString GetPluginName() override { return GetPluginNameStatic(); }

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

@@ -135,7 +135,7 @@ class PluginProperties : public Properties {
     m_collection_sp->Initialize(g_properties);
   }
 
-  virtual ~PluginProperties() {}
+  ~PluginProperties() override {}
 
   uint64_t GetPacketTimeout() {
     const uint32_t idx = ePropertyPacketTimeout;
@@ -5158,7 +5158,7 @@ class CommandObjectProcessGDBRemoteSpeedTest : public CommandObjectParsed {
     m_option_group.Finalize();
   }
 
-  ~CommandObjectProcessGDBRemoteSpeedTest() {}
+  ~CommandObjectProcessGDBRemoteSpeedTest() override {}
 
   Options *GetOptions() override { return &m_option_group; }
 
@@ -5209,7 +5209,7 @@ class CommandObjectProcessGDBRemotePacketHistory : public CommandObjectParsed {
       : CommandObjectParsed(interpreter, "process plugin packet history",
                             "Dumps the packet history buffer. ", NULL) {}
 
-  ~CommandObjectProcessGDBRemotePacketHistory() {}
+  ~CommandObjectProcessGDBRemotePacketHistory() override {}
 
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     const size_t argc = command.GetArgumentCount();
@@ -5240,7 +5240,7 @@ class CommandObjectProcessGDBRemotePacketXferSize : public CommandObjectParsed {
             "Maximum size that lldb will try to read/write one one chunk.",
             NULL) {}
 
-  ~CommandObjectProcessGDBRemotePacketXferSize() {}
+  ~CommandObjectProcessGDBRemotePacketXferSize() override {}
 
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     const size_t argc = command.GetArgumentCount();
@@ -5282,7 +5282,7 @@ class CommandObjectProcessGDBRemotePacketSend : public CommandObjectParsed {
                             "stripped from the result.",
                             NULL) {}
 
-  ~CommandObjectProcessGDBRemotePacketSend() {}
+  ~CommandObjectProcessGDBRemotePacketSend() override {}
 
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     const size_t argc = command.GetArgumentCount();
@@ -5333,7 +5333,7 @@ class CommandObjectProcessGDBRemotePacketMonitor : public CommandObjectRaw {
                          "encoded into a valid 'qRcmd' packet, sent and the "
                          "response will be printed.") {}
 
-  ~CommandObjectProcessGDBRemotePacketMonitor() {}
+  ~CommandObjectProcessGDBRemotePacketMonitor() override {}
 
   bool DoExecute(llvm::StringRef command,
                  CommandReturnObject &result) override {
@@ -5397,7 +5397,7 @@ class CommandObjectProcessGDBRemotePacket : public CommandObjectMultiword {
                        interpreter)));
   }
 
-  ~CommandObjectProcessGDBRemotePacket() {}
+  ~CommandObjectProcessGDBRemotePacket() override {}
 };
 
 class CommandObjectMultiwordProcessGDBRemote : public CommandObjectMultiword {
@@ -5412,7 +5412,7 @@ class CommandObjectMultiwordProcessGDBRemote : public CommandObjectMultiword {
         CommandObjectSP(new CommandObjectProcessGDBRemotePacket(interpreter)));
   }
 
-  ~CommandObjectMultiwordProcessGDBRemote() {}
+  ~CommandObjectMultiwordProcessGDBRemote() override {}
 };
 
 CommandObject *ProcessGDBRemote::GetPluginCommandObject() {

lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

@@ -683,7 +683,7 @@ class CommandObjectProcessMinidumpDump : public CommandObjectParsed {
     m_option_group.Finalize();
   }
 
-  ~CommandObjectProcessMinidumpDump() {}
+  ~CommandObjectProcessMinidumpDump() override {}
 
   Options *GetOptions() override { return &m_option_group; }
 
@@ -813,7 +813,7 @@ class CommandObjectMultiwordProcessMinidump : public CommandObjectMultiword {
         CommandObjectSP(new CommandObjectProcessMinidumpDump(interpreter)));
   }
 
-  ~CommandObjectMultiwordProcessMinidump() {}
+  ~CommandObjectMultiwordProcessMinidump() override {}
 };
 
 CommandObject *ProcessMinidump::GetPluginCommandObject() {

lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp

@@ -141,7 +141,7 @@ class StructuredDataDarwinLogProperties : public Properties {
     m_collection_sp->Initialize(g_properties);
   }
 
-  virtual ~StructuredDataDarwinLogProperties() {}
+  ~StructuredDataDarwinLogProperties() override {}
 
   bool GetEnableOnStartup() const {
     const uint32_t idx = ePropertyEnableOnStartup;

lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.h

@@ -65,7 +65,7 @@ class StructuredDataDarwinLog : public StructuredDataPlugin {
 
   void ModulesDidLoad(Process &process, ModuleList &module_list) override;
 
-  ~StructuredDataDarwinLog();
+  ~StructuredDataDarwinLog() override;
 
 private:
   // Private constructors

lldb/source/Symbol/ClangASTContext.cpp

@@ -951,7 +951,7 @@ class NullDiagnosticConsumer : public DiagnosticConsumer {
   }
 
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
-                        const clang::Diagnostic &info) {
+                        const clang::Diagnostic &info) override {
     if (m_log) {
       llvm::SmallVector<char, 32> diag_str(10);
       info.FormatDiagnostic(diag_str);

lldb/source/Symbol/PostfixExpression.cpp

@@ -124,23 +124,23 @@ class DWARFCodegen : public Visitor<> {
   using Visitor<>::Dispatch;
 
 private:
-  void Visit(BinaryOpNode &binary, Node *&);
+  void Visit(BinaryOpNode &binary, Node *&) override;
 
-  void Visit(InitialValueNode &val, Node *&);
+  void Visit(InitialValueNode &val, Node *&) override;
 
-  void Visit(IntegerNode &integer, Node *&) {
+  void Visit(IntegerNode &integer, Node *&) override {
     m_out_stream.PutHex8(DW_OP_constu);
     m_out_stream.PutULEB128(integer.GetValue());
     ++m_stack_depth;
   }
 
-  void Visit(RegisterNode &reg, Node *&);
+  void Visit(RegisterNode &reg, Node *&) override;
 
-  void Visit(SymbolNode &symbol, Node *&) {
+  void Visit(SymbolNode &symbol, Node *&) override {
     llvm_unreachable("Symbols should have been resolved by now!");
   }
 
-  void Visit(UnaryOpNode &unary, Node *&);
+  void Visit(UnaryOpNode &unary, Node *&) override;
 
   Stream &m_out_stream;