Applying clang-tidy modernize-use-override over LLDB

[shafik]

Summary

Applied clang-tidy modernize-use-override over LLDB and added it to the LLDB .clang-tidy config.

[JDevlieghere]

LGTM modulo the redundant comment

lldb/unittests/Target/RemoteAwarePlatformTest.cpp

40–41

[labath]

The reason for the funny /*override*/ thingy in the mock classes is that it is impossible (in the gmock version that we use) to add the override keyword to the methods overridden by the MOCK macros. Then, having override keywords on hand-written methods triggered -Winconsistent-missing-override.

Or at least it used to -- have you checked that these changes don't introduce any new compiler warnings?

[shafik]

In D123340#3438391, @labath wrote:

The reason for the funny /*override*/ thingy in the mock classes is that it is impossible (in the gmock version that we use) to add the override keyword to the methods overridden by the MOCK macros. Then, having override keywords on hand-written methods triggered -Winconsistent-missing-override.

Or at least it used to -- have you checked that these changes don't introduce any new compiler warnings?

I missed that, it does indeed trigger the same warning, I guess I will add a NOLINT

[shafik]

-Removed override RemoteAwarePlatformTest.cpp and added NOLINT

[labath]

Yeah, I suppose we can do that. If this turns out to be more widespread, then I'd rather disable the tidy check (or perhaps the clang warning) for the unittest files instead.

lldb/.clang-tidy

-Checks: '-readability-identifier-naming,modernize-use-default-member-init,modernize-use-equals-default'
+Checks: '-readability-identifier-naming,modernize-use-default-member-init,modernize-use-equals-default,modernize-use-override'
 InheritParentConfig: true
 CheckOptions:
   - key:             modernize-use-default-member-init.IgnoreMacros
     value:           '1'
   - key:             modernize-use-default-member-init.UseAssignment
     value:           '1'

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

public:
   explicit CompleteTagDeclsScope(ClangASTImporter &importer,
                             clang::ASTContext *dst_ctx,
                             clang::ASTContext *src_ctx)
       : m_delegate(importer.GetDelegate(dst_ctx, src_ctx)), m_dst_ctx(dst_ctx),
         m_src_ctx(src_ctx), importer(importer) {
     m_delegate->SetImportListener(this);
   }
 
-  virtual ~CompleteTagDeclsScope() {
+  ~CompleteTagDeclsScope() override {
     ClangASTImporter::ASTContextMetadataSP to_context_md =
         importer.GetContextMetadata(m_dst_ctx);
 
     // Complete all decls we collected until now.
     while (!m_decls_to_complete.empty()) {
       NamedDecl *decl = m_decls_to_complete.pop_back_val();
       m_decls_already_completed.insert(decl);
 

lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp

static ConstString &GetSettingName() {
     return g_setting_name;
   }
 
   PlatformDarwinKernelProperties() : Properties() {
     m_collection_sp = std::make_shared<OptionValueProperties>(GetSettingName());
     m_collection_sp->Initialize(g_platformdarwinkernel_properties);
   }
 
-  virtual ~PlatformDarwinKernelProperties() = default;
+  ~PlatformDarwinKernelProperties() override = default;
 
   FileSpecList GetKextDirectories() const {
     const uint32_t idx = ePropertyKextDirectories;
     const OptionValueFileSpecList *option_value =
         m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpecList(
             NULL, false, idx);
     assert(option_value);
     return option_value->GetCurrentValue();

lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp

static ConstString GetSettingName() {
     return ConstString(ProcessKDP::GetPluginNameStatic());
   }
 
   PluginProperties() : Properties() {
     m_collection_sp = std::make_shared<OptionValueProperties>(GetSettingName());
     m_collection_sp->Initialize(g_processkdp_properties);
   }
 
-  virtual ~PluginProperties() = default;
+  ~PluginProperties() override = default;
 
   uint64_t GetPacketTimeout() {
     const uint32_t idx = ePropertyKDPPacketTimeout;
     return m_collection_sp->GetPropertyAtIndexAsUInt64(
         NULL, idx, g_processkdp_properties[idx].default_uint_value);
   }
 };
 
CommandObjectProcessKDPPacketSend(CommandInterpreter &interpreter)
                       "Specify packet payload bytes as a hex ASCII string with "
                       "no spaces or hex prefixes.",
                       NULL) {
     m_option_group.Append(&m_command_byte, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
     m_option_group.Append(&m_packet_data, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
     m_option_group.Finalize();
   }
 
-  ~CommandObjectProcessKDPPacketSend() = default;
+  ~CommandObjectProcessKDPPacketSend() override = default;
 
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     const size_t argc = command.GetArgumentCount();
     if (argc == 0) {
       if (!m_command_byte.GetOptionValue().OptionWasSet()) {
         result.AppendError(
             "the --command option must be set to a valid command byte");
       } else {
CommandObjectProcessKDPPacket(CommandInterpreter &interpreter)
       : CommandObjectMultiword(interpreter, "process plugin packet",
                                "Commands that deal with KDP remote packets.",
                                NULL) {
     LoadSubCommand(
         "send",
         CommandObjectSP(new CommandObjectProcessKDPPacketSend(interpreter)));
   }
 
-  ~CommandObjectProcessKDPPacket() = default;
+  ~CommandObjectProcessKDPPacket() override = default;
 };
 
 class CommandObjectMultiwordProcessKDP : public CommandObjectMultiword {
 public:
   CommandObjectMultiwordProcessKDP(CommandInterpreter &interpreter)
       : CommandObjectMultiword(
             interpreter, "process plugin",
             "Commands for operating on a ProcessKDP process.",
             "process plugin <subcommand> [<subcommand-options>]") {
     LoadSubCommand("packet", CommandObjectSP(new CommandObjectProcessKDPPacket(
                                  interpreter)));
   }
 
-  ~CommandObjectMultiwordProcessKDP() = default;
+  ~CommandObjectMultiwordProcessKDP() override = default;
 };
 
 CommandObject *ProcessKDP::GetPluginCommandObject() {
   if (!m_command_sp)
     m_command_sp = std::make_shared<CommandObjectMultiwordProcessKDP>(
         GetTarget().GetDebugger().GetCommandInterpreter());
   return m_command_sp.get();
 }

lldb/unittests/API/SBCommandInterpreterTest.cpp

protected:
   SBCommandInterpreter m_interp;
 };
 
 class DummyCommand : public SBCommandPluginInterface {
 public:
   DummyCommand(const char *message) : m_message(message) {}
 
   bool DoExecute(SBDebugger dbg, char **command,
-                 SBCommandReturnObject &result) {
+                 SBCommandReturnObject &result) override {
     result.PutCString(m_message.c_str());
     result.SetStatus(eReturnStatusSuccessFinishResult);
     return result.Succeeded();
   }
 
 private:
   std::string m_message;
 };

lldb/unittests/Interpreter/TestCommandPaths.cpp

 public:
   CommandObjectLeaf(CommandInterpreter &interpreter)
       : CommandObjectParsed(interpreter, "dummy subcommand leaf",
                             "Does nothing", "dummy subcommand leaf") {
     SetIsUserCommand(true);
   }
 
 protected:
-  virtual bool DoExecute(Args &command, CommandReturnObject &result) {
+  bool DoExecute(Args &command, CommandReturnObject &result) override {
     result.SetStatus(eReturnStatusSuccessFinishResult);
     result.AppendMessage("I did nothing");
     return true;
   }
 };
 
 class CommandObjectMultiwordSubDummy : public CommandObjectMultiword {
 public:

lldb/unittests/Interpreter/TestOptionValue.cpp

public:
   }
 
   OptionValueFileSpecList *GetFileList() {
     return GetPropertyAtIndexAsOptionValueFileSpecList(nullptr, true,
                                                        m_file_list_index);
   }
 
 private:
-  lldb::OptionValueSP Clone() const {
+  lldb::OptionValueSP Clone() const override {
     return std::make_shared<TestProperties>(*this);
   }
 
   uint32_t m_dict_index = 0;
   uint32_t m_file_list_index = 1;
 };
 
 // Test a user-defined propery class.

lldb/unittests/Target/RemoteAwarePlatformTest.cpp

MOCK_METHOD4(Attach,
                ProcessSP(ProcessAttachInfo &, Debugger &, Target *, Status &));
   MOCK_METHOD0(CalculateTrapHandlerSymbolNames, void());
 
   MOCK_METHOD2(ResolveRemoteExecutable,
                std::pair<Status, ModuleSP>(const ModuleSpec &,
                                            const FileSpecList *));
   Status ResolveRemoteExecutable(
       const ModuleSpec &module_spec, lldb::ModuleSP &exe_module_sp,
-      const FileSpecList *module_search_paths_ptr) /*override*/ {
+      const FileSpecList *module_search_paths_ptr) /*override*/
+  { // NOLINT(modernize-use-override)
     auto pair = ResolveRemoteExecutable(module_spec, module_search_paths_ptr);
     exe_module_sp = pair.second;
     return pair.first;
   }
 
   void SetRemotePlatform(lldb::PlatformSP platform) {
     m_remote_platform_sp = platform;
   }