jingham created this revision.
jingham added reviewers: aprantl, shafik.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

  Many of the API's that returned BreakpointOptions always returned valid ones.
  However, internally the BreakpointLocations usually have null 
BreakpointOptions, since they
  use their owner's options until an option is set specifically on the location.
  So the original code used pointers & unique_ptr everywhere for consistency.
  But that made the code hard to reason about from the outside, since it wasn't 
clear 
  when you did and didn't have to validate the pointers.
  
  This patch changes the code so that everywhere an API is guaranteed to
  return a non-null BreakpointOption, it returns it as a reference to make
  that clear.
  
  It also changes the Breakpoint to hold a BreakpointOption
  member where it previously had a UP.  Since we were always filling the UP
  in the Breakpoint constructor, having the UP wasn't helping anything.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104162

Files:
  lldb/include/lldb/Breakpoint/Breakpoint.h
  lldb/include/lldb/Breakpoint/BreakpointLocation.h
  lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h
  lldb/include/lldb/Breakpoint/BreakpointSite.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/source/API/SBBreakpoint.cpp
  lldb/source/API/SBBreakpointLocation.cpp
  lldb/source/API/SBBreakpointName.cpp
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Breakpoint/BreakpointLocation.cpp
  lldb/source/Breakpoint/BreakpointLocationCollection.cpp
  lldb/source/Breakpoint/BreakpointName.cpp
  lldb/source/Breakpoint/BreakpointOptions.cpp
  lldb/source/Breakpoint/BreakpointSite.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/source/Target/StopInfo.cpp

Index: lldb/source/Target/StopInfo.cpp
===================================================================
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -125,7 +125,7 @@
       BreakpointSiteSP bp_site_sp(
           process_sp->GetBreakpointSiteList().FindByID(m_value));
       if (bp_site_sp)
-        return bp_site_sp->ValidForThisThread(&thread);
+        return bp_site_sp->ValidForThisThread(thread);
     }
     return false;
   }
@@ -413,7 +413,7 @@
             // The breakpoint site may have many locations associated with it,
             // not all of them valid for this thread.  Skip the ones that
             // aren't:
-            if (!bp_loc_sp->ValidForThisThread(thread_sp.get())) {
+            if (!bp_loc_sp->ValidForThisThread(*thread_sp)) {
               if (log) {
                 LLDB_LOGF(log,
                           "Breakpoint %s hit on thread 0x%llx but it was not "
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -244,7 +244,7 @@
   std::unique_ptr<ScriptInterpreterLocker> AcquireInterpreterLock() override;
 
   void CollectDataForBreakpointCommandCallback(
-      std::vector<BreakpointOptions *> &bp_options_vec,
+      std::vector<std::reference_wrapper<BreakpointOptions>> &bp_options_vec,
       CommandReturnObject &result) override;
 
   void
@@ -252,20 +252,19 @@
                                           CommandReturnObject &result) override;
 
   /// Set the callback body text into the callback for the breakpoint.
-  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  Status SetBreakpointCommandCallback(BreakpointOptions &bp_options,
                                       const char *callback_body) override;
 
   Status SetBreakpointCommandCallbackFunction(
-      BreakpointOptions *bp_options,
-      const char *function_name,
+      BreakpointOptions &bp_options, const char *function_name,
       StructuredData::ObjectSP extra_args_sp) override;
 
   /// This one is for deserialization:
   Status SetBreakpointCommandCallback(
-      BreakpointOptions *bp_options,
+      BreakpointOptions &bp_options,
       std::unique_ptr<BreakpointOptions::CommandData> &data_up) override;
 
-  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  Status SetBreakpointCommandCallback(BreakpointOptions &bp_options,
                                       const char *command_body_text,
                                       StructuredData::ObjectSP extra_args_sp,
                                       bool uses_extra_args);
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -635,11 +635,10 @@
   case eIOHandlerNone:
     break;
   case eIOHandlerBreakpoint: {
-    std::vector<BreakpointOptions *> *bp_options_vec =
-        (std::vector<BreakpointOptions *> *)io_handler.GetUserData();
-    for (auto bp_options : *bp_options_vec) {
-      if (!bp_options)
-        continue;
+    std::vector<std::reference_wrapper<BreakpointOptions>> *bp_options_vec =
+        (std::vector<std::reference_wrapper<BreakpointOptions>> *)
+            io_handler.GetUserData();
+    for (BreakpointOptions &bp_options : *bp_options_vec) {
 
       auto data_up = std::make_unique<CommandDataPython>();
       if (!data_up)
@@ -653,7 +652,7 @@
               .Success()) {
         auto baton_sp = std::make_shared<BreakpointOptions::CommandBaton>(
             std::move(data_up));
-        bp_options->SetCallback(
+        bp_options.SetCallback(
             ScriptInterpreterPythonImpl::BreakpointCallbackFunction, baton_sp);
       } else if (!batch_mode) {
         StreamFileSP error_sp = io_handler.GetErrorStreamFileSP();
@@ -1226,7 +1225,7 @@
 }
 
 void ScriptInterpreterPythonImpl::CollectDataForBreakpointCommandCallback(
-    std::vector<BreakpointOptions *> &bp_options_vec,
+    std::vector<std::reference_wrapper<BreakpointOptions>> &bp_options_vec,
     CommandReturnObject &result) {
   m_active_io_handler = eIOHandlerBreakpoint;
   m_debugger.GetCommandInterpreter().GetPythonCommandsFromIOHandler(
@@ -1241,7 +1240,7 @@
 }
 
 Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallbackFunction(
-    BreakpointOptions *bp_options, const char *function_name,
+    BreakpointOptions &bp_options, const char *function_name,
     StructuredData::ObjectSP extra_args_sp) {
   Status error;
   // For now just cons up a oneliner that calls the provided function.
@@ -1283,7 +1282,7 @@
 }
 
 Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
-    BreakpointOptions *bp_options,
+    BreakpointOptions &bp_options,
     std::unique_ptr<BreakpointOptions::CommandData> &cmd_data_up) {
   Status error;
   error = GenerateBreakpointCommandCallbackData(cmd_data_up->user_source,
@@ -1294,21 +1293,20 @@
   }
   auto baton_sp =
       std::make_shared<BreakpointOptions::CommandBaton>(std::move(cmd_data_up));
-  bp_options->SetCallback(
+  bp_options.SetCallback(
       ScriptInterpreterPythonImpl::BreakpointCallbackFunction, baton_sp);
   return error;
 }
 
 Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
-    BreakpointOptions *bp_options, const char *command_body_text) {
+    BreakpointOptions &bp_options, const char *command_body_text) {
   return SetBreakpointCommandCallback(bp_options, command_body_text, {},false);
 }
 
 // Set a Python one-liner as the callback for the breakpoint.
 Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
-    BreakpointOptions *bp_options, const char *command_body_text,
-    StructuredData::ObjectSP extra_args_sp,
-    bool uses_extra_args) {
+    BreakpointOptions &bp_options, const char *command_body_text,
+    StructuredData::ObjectSP extra_args_sp, bool uses_extra_args) {
   auto data_up = std::make_unique<CommandDataPython>(extra_args_sp);
   // Split the command_body_text into lines, and pass that to
   // GenerateBreakpointCommandCallbackData.  That will wrap the body in an
@@ -1322,7 +1320,7 @@
   if (error.Success()) {
     auto baton_sp =
         std::make_shared<BreakpointOptions::CommandBaton>(std::move(data_up));
-    bp_options->SetCallback(
+    bp_options.SetCallback(
         ScriptInterpreterPythonImpl::BreakpointCallbackFunction, baton_sp);
     return error;
   }
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -9,6 +9,8 @@
 #ifndef liblldb_ScriptInterpreterLua_h_
 #define liblldb_ScriptInterpreterLua_h_
 
+#include <vector>
+
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Utility/Status.h"
@@ -72,21 +74,21 @@
   llvm::Error LeaveSession();
 
   void CollectDataForBreakpointCommandCallback(
-      std::vector<BreakpointOptions *> &bp_options_vec,
+      std::vector<std::reference_wrapper<BreakpointOptions>> &bp_options_vec,
       CommandReturnObject &result) override;
 
-  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  Status SetBreakpointCommandCallback(BreakpointOptions &bp_options,
                                       const char *command_body_text) override;
 
   Status SetBreakpointCommandCallbackFunction(
-      BreakpointOptions *bp_options, const char *function_name,
+      BreakpointOptions &bp_options, const char *function_name,
       StructuredData::ObjectSP extra_args_sp) override;
 
 private:
   std::unique_ptr<Lua> m_lua;
   bool m_session_is_active = false;
 
-  Status RegisterBreakpointCallback(BreakpointOptions *bp_options,
+  Status RegisterBreakpointCallback(BreakpointOptions &bp_options,
                                     const char *command_body_text,
                                     StructuredData::ObjectSP extra_args_sp);
 };
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -98,9 +98,10 @@
                               std::string &data) override {
     switch (m_active_io_handler) {
     case eIOHandlerBreakpoint: {
-      auto *bp_options_vec = static_cast<std::vector<BreakpointOptions *> *>(
-          io_handler.GetUserData());
-      for (auto *bp_options : *bp_options_vec) {
+      auto *bp_options_vec =
+          static_cast<std::vector<std::reference_wrapper<BreakpointOptions>> *>(
+              io_handler.GetUserData());
+      for (BreakpointOptions &bp_options : *bp_options_vec) {
         Status error = m_script_interpreter.SetBreakpointCommandCallback(
             bp_options, data.c_str());
         if (error.Fail())
@@ -276,7 +277,7 @@
 }
 
 void ScriptInterpreterLua::CollectDataForBreakpointCommandCallback(
-    std::vector<BreakpointOptions *> &bp_options_vec,
+    std::vector<std::reference_wrapper<BreakpointOptions>> &bp_options_vec,
     CommandReturnObject &result) {
   IOHandlerSP io_handler_sp(
       new IOHandlerLuaInterpreter(m_debugger, *this, eIOHandlerBreakpoint));
@@ -285,7 +286,7 @@
 }
 
 Status ScriptInterpreterLua::SetBreakpointCommandCallbackFunction(
-    BreakpointOptions *bp_options, const char *function_name,
+    BreakpointOptions &bp_options, const char *function_name,
     StructuredData::ObjectSP extra_args_sp) {
   const char *fmt_str = "return {0}(frame, bp_loc, ...)";
   std::string oneliner = llvm::formatv(fmt_str, function_name).str();
@@ -294,12 +295,12 @@
 }
 
 Status ScriptInterpreterLua::SetBreakpointCommandCallback(
-    BreakpointOptions *bp_options, const char *command_body_text) {
+    BreakpointOptions &bp_options, const char *command_body_text) {
   return RegisterBreakpointCallback(bp_options, command_body_text, {});
 }
 
 Status ScriptInterpreterLua::RegisterBreakpointCallback(
-    BreakpointOptions *bp_options, const char *command_body_text,
+    BreakpointOptions &bp_options, const char *command_body_text,
     StructuredData::ObjectSP extra_args_sp) {
   Status error;
   auto data_up = std::make_unique<CommandDataLua>(extra_args_sp);
@@ -308,8 +309,8 @@
     return error;
   auto baton_sp =
       std::make_shared<BreakpointOptions::CommandBaton>(std::move(data_up));
-  bp_options->SetCallback(ScriptInterpreterLua::BreakpointCallbackFunction,
-                          baton_sp);
+  bp_options.SetCallback(ScriptInterpreterLua::BreakpointCallbackFunction,
+                         baton_sp);
   return error;
 }
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1832,8 +1832,7 @@
               // If the current pc is a breakpoint site then the StopInfo
               // should be set to Breakpoint Otherwise, it will be set to
               // Trace.
-              if (bp_site_sp &&
-                  bp_site_sp->ValidForThisThread(thread_sp.get())) {
+              if (bp_site_sp && bp_site_sp->ValidForThisThread(*thread_sp)) {
                 thread_sp->SetStopInfo(
                     StopInfo::CreateStopReasonWithBreakpointSiteID(
                         *thread_sp, bp_site_sp->GetID()));
@@ -1853,7 +1852,7 @@
                 // breakpoint here, that will be taken care of when the thread
                 // resumes and notices that there's a breakpoint under the pc.
                 handled = true;
-                if (bp_site_sp->ValidForThisThread(thread_sp.get())) {
+                if (bp_site_sp->ValidForThisThread(*thread_sp)) {
                   thread_sp->SetStopInfo(
                       StopInfo::CreateStopReasonWithBreakpointSiteID(
                           *thread_sp, bp_site_sp->GetID()));
@@ -1919,7 +1918,7 @@
             // as such. This can happen when the thread is involuntarily
             // interrupted (e.g. due to stops on other threads) just as it is
             // about to execute the breakpoint instruction.
-            if (bp_site_sp && bp_site_sp->ValidForThisThread(thread_sp.get())) {
+            if (bp_site_sp && bp_site_sp->ValidForThisThread(*thread_sp)) {
               thread_sp->SetStopInfo(
                   StopInfo::CreateStopReasonWithBreakpointSiteID(
                       *thread_sp, bp_site_sp->GetID()));
@@ -1944,7 +1943,7 @@
                 // reason.  We don't need to worry about stepping over the
                 // breakpoint here, that will be taken care of when the thread
                 // resumes and notices that there's a breakpoint under the pc.
-                if (bp_site_sp->ValidForThisThread(thread_sp.get())) {
+                if (bp_site_sp->ValidForThisThread(*thread_sp)) {
                   if (m_breakpoint_pc_offset != 0)
                     thread_sp->GetRegisterContext()->SetPC(pc);
                   thread_sp->SetStopInfo(
Index: lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
===================================================================
--- lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -509,7 +509,7 @@
         // operating system thread ID, so we can't make any assumptions about
         // the thread ID so we must always report the breakpoint regardless
         // of the thread.
-        if (bp_site_sp->ValidForThisThread(&thread) ||
+        if (bp_site_sp->ValidForThisThread(thread) ||
             thread.GetProcess()->GetOperatingSystem() != nullptr)
           return StopInfo::CreateStopReasonWithBreakpointSiteID(
               thread, bp_site_sp->GetID());
Index: lldb/source/Interpreter/ScriptInterpreter.cpp
===================================================================
--- lldb/source/Interpreter/ScriptInterpreter.cpp
+++ lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -34,7 +34,7 @@
           std::move(scripted_process_interface_up)) {}
 
 void ScriptInterpreter::CollectDataForBreakpointCommandCallback(
-    std::vector<BreakpointOptions *> &bp_options_vec,
+    std::vector<std::reference_wrapper<BreakpointOptions>> &bp_options_vec,
     CommandReturnObject &result) {
   result.SetStatus(eReturnStatusFailed);
   result.AppendError(
@@ -97,10 +97,10 @@
 }
 
 Status ScriptInterpreter::SetBreakpointCommandCallback(
-    std::vector<BreakpointOptions *> &bp_options_vec,
+    std::vector<std::reference_wrapper<BreakpointOptions>> &bp_options_vec,
     const char *callback_text) {
   Status return_error;
-  for (BreakpointOptions *bp_options : bp_options_vec) {
+  for (BreakpointOptions &bp_options : bp_options_vec) {
     return_error = SetBreakpointCommandCallback(bp_options, callback_text);
     if (return_error.Success())
       break;
@@ -109,10 +109,10 @@
 }
 
 Status ScriptInterpreter::SetBreakpointCommandCallbackFunction(
-    std::vector<BreakpointOptions *> &bp_options_vec, const char *function_name,
-    StructuredData::ObjectSP extra_args_sp) {
+    std::vector<std::reference_wrapper<BreakpointOptions>> &bp_options_vec,
+    const char *function_name, StructuredData::ObjectSP extra_args_sp) {
   Status error;
-  for (BreakpointOptions *bp_options : bp_options_vec) {
+  for (BreakpointOptions &bp_options : bp_options_vec) {
     error = SetBreakpointCommandCallbackFunction(bp_options, function_name,
                                                  extra_args_sp);
     if (!error.Success())
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===================================================================
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -3952,7 +3952,7 @@
               false,               // request_hardware
               eLazyBoolCalculate); // move_to_nearest_code
           // Make breakpoint one shot
-          bp_sp->GetOptions()->SetOneShot(true);
+          bp_sp->GetOptions().SetOneShot(true);
           exe_ctx.GetProcessRef().Resume();
         }
       } else if (m_selected_line < GetNumDisassemblyLines()) {
@@ -3968,7 +3968,7 @@
               false,  // internal
               false); // request_hardware
           // Make breakpoint one shot
-          bp_sp->GetOptions()->SetOneShot(true);
+          bp_sp->GetOptions().SetOneShot(true);
           exe_ctx.GetProcessRef().Resume();
         }
       }
Index: lldb/source/Commands/CommandObjectBreakpointCommand.cpp
===================================================================
--- lldb/source/Commands/CommandObjectBreakpointCommand.cpp
+++ lldb/source/Commands/CommandObjectBreakpointCommand.cpp
@@ -245,20 +245,18 @@
                               std::string &line) override {
     io_handler.SetIsDone(true);
 
-    std::vector<BreakpointOptions *> *bp_options_vec =
-        (std::vector<BreakpointOptions *> *)io_handler.GetUserData();
-    for (BreakpointOptions *bp_options : *bp_options_vec) {
-      if (!bp_options)
-        continue;
-
+    std::vector<std::reference_wrapper<BreakpointOptions>> *bp_options_vec =
+        (std::vector<std::reference_wrapper<BreakpointOptions>> *)
+            io_handler.GetUserData();
+    for (BreakpointOptions &bp_options : *bp_options_vec) {
       auto cmd_data = std::make_unique<BreakpointOptions::CommandData>();
       cmd_data->user_source.SplitIntoLines(line.c_str(), line.size());
-      bp_options->SetCommandDataCallback(cmd_data);
+      bp_options.SetCommandDataCallback(cmd_data);
     }
   }
 
   void CollectDataForBreakpointCommandCallback(
-      std::vector<BreakpointOptions *> &bp_options_vec,
+      std::vector<std::reference_wrapper<BreakpointOptions>> &bp_options_vec,
       CommandReturnObject &result) {
     m_interpreter.GetLLDBCommandsFromIOHandler(
         "> ",             // Prompt
@@ -268,16 +266,16 @@
   }
 
   /// Set a one-liner as the callback for the breakpoint.
-  void
-  SetBreakpointCommandCallback(std::vector<BreakpointOptions *> &bp_options_vec,
-                               const char *oneliner) {
-    for (auto bp_options : bp_options_vec) {
+  void SetBreakpointCommandCallback(
+      std::vector<std::reference_wrapper<BreakpointOptions>> &bp_options_vec,
+      const char *oneliner) {
+    for (BreakpointOptions &bp_options : bp_options_vec) {
       auto cmd_data = std::make_unique<BreakpointOptions::CommandData>();
 
       cmd_data->user_source.AppendString(oneliner);
       cmd_data->stop_on_error = m_options.m_stop_on_error;
 
-      bp_options->SetCommandDataCallback(cmd_data);
+      bp_options.SetCommandDataCallback(cmd_data);
     }
   }
 
@@ -400,20 +398,17 @@
         if (cur_bp_id.GetBreakpointID() != LLDB_INVALID_BREAK_ID) {
           Breakpoint *bp =
               target.GetBreakpointByID(cur_bp_id.GetBreakpointID()).get();
-          BreakpointOptions *bp_options = nullptr;
           if (cur_bp_id.GetLocationID() == LLDB_INVALID_BREAK_ID) {
             // This breakpoint does not have an associated location.
-            bp_options = bp->GetOptions();
+            m_bp_options_vec.push_back(bp->GetOptions());
           } else {
             BreakpointLocationSP bp_loc_sp(
                 bp->FindLocationByID(cur_bp_id.GetLocationID()));
             // This breakpoint does have an associated location. Get its
             // breakpoint options.
             if (bp_loc_sp)
-              bp_options = bp_loc_sp->GetLocationOptions();
+              m_bp_options_vec.push_back(bp_loc_sp->GetLocationOptions());
           }
-          if (bp_options)
-            m_bp_options_vec.push_back(bp_options);
         }
       }
 
@@ -456,9 +451,10 @@
   OptionGroupPythonClassWithDict m_func_options;
   OptionGroupOptions m_all_options;
 
-  std::vector<BreakpointOptions *> m_bp_options_vec; // This stores the
-                                                     // breakpoint options that
-                                                     // we are currently
+  std::vector<std::reference_wrapper<BreakpointOptions>>
+      m_bp_options_vec; // This stores the
+                        // breakpoint options that
+                        // we are currently
   // collecting commands for.  In the CollectData... calls we need to hand this
   // off to the IOHandler, which may run asynchronously. So we have to have
   // some way to keep it alive, and not leak it. Making it an ivar of the
@@ -678,9 +674,9 @@
               baton =
                   bp_loc_sp
                       ->GetOptionsSpecifyingKind(BreakpointOptions::eCallback)
-                      ->GetBaton();
+                      .GetBaton();
             else
-              baton = bp->GetOptions()->GetBaton();
+              baton = bp->GetOptions().GetBaton();
 
             if (baton) {
               result.GetOutputStream().Printf("Breakpoint %s:\n",
Index: lldb/source/Commands/CommandObjectBreakpoint.cpp
===================================================================
--- lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -713,7 +713,7 @@
 
     // Now set the various options that were passed in:
     if (bp_sp) {
-      bp_sp->GetOptions()->CopyOverSetOptions(m_bp_opts.GetBreakpointOptions());
+      bp_sp->GetOptions().CopyOverSetOptions(m_bp_opts.GetBreakpointOptions());
 
       if (!m_options.m_breakpoint_names.empty()) {
         Status name_error;
@@ -863,10 +863,10 @@
             BreakpointLocation *location =
                 bp->FindLocationByID(cur_bp_id.GetLocationID()).get();
             if (location)
-              location->GetLocationOptions()->CopyOverSetOptions(
+              location->GetLocationOptions().CopyOverSetOptions(
                   m_bp_opts.GetBreakpointOptions());
           } else {
-            bp->GetOptions()->CopyOverSetOptions(
+            bp->GetOptions().CopyOverSetOptions(
                 m_bp_opts.GetBreakpointOptions());
           }
         }
@@ -1769,7 +1769,7 @@
         bp_name->SetHelp(m_bp_id.m_help_string.GetStringValue().str().c_str());
 
       if (bp_sp)
-        target.ConfigureBreakpointName(*bp_name, *bp_sp->GetOptions(),
+        target.ConfigureBreakpointName(*bp_name, bp_sp->GetOptions(),
                                        m_access_options.GetPermissions());
       else
         target.ConfigureBreakpointName(*bp_name,
Index: lldb/source/Breakpoint/BreakpointSite.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointSite.cpp
+++ lldb/source/Breakpoint/BreakpointSite.cpp
@@ -144,7 +144,7 @@
   return m_owners.GetByIndex(index);
 }
 
-bool BreakpointSite::ValidForThisThread(Thread *thread) {
+bool BreakpointSite::ValidForThisThread(Thread &thread) {
   std::lock_guard<std::recursive_mutex> guard(m_owners_mutex);
   return m_owners.ValidForThisThread(thread);
 }
Index: lldb/source/Breakpoint/BreakpointOptions.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointOptions.cpp
+++ lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -332,7 +332,7 @@
       }
       Status script_error;
       script_error =
-          interp->SetBreakpointCommandCallback(bp_options.get(), cmd_data_up);
+          interp->SetBreakpointCommandCallback(*bp_options, cmd_data_up);
       if (script_error.Fail()) {
         error.SetErrorStringWithFormat("Error generating script callback: %s.",
                                        error.AsCString());
Index: lldb/source/Breakpoint/BreakpointName.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointName.cpp
+++ lldb/source/Breakpoint/BreakpointName.cpp
@@ -81,6 +81,6 @@
 
 void BreakpointName::ConfigureBreakpoint(lldb::BreakpointSP bp_sp)
 {
-   bp_sp->GetOptions()->CopyOverSetOptions(GetOptions());
-   bp_sp->GetPermissions().MergeInto(GetPermissions());
+  bp_sp->GetOptions().CopyOverSetOptions(GetOptions());
+  bp_sp->GetPermissions().MergeInto(GetPermissions());
 }
Index: lldb/source/Breakpoint/BreakpointLocationCollection.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointLocationCollection.cpp
+++ lldb/source/Breakpoint/BreakpointLocationCollection.cpp
@@ -134,7 +134,7 @@
   return shouldStop;
 }
 
-bool BreakpointLocationCollection::ValidForThisThread(Thread *thread) {
+bool BreakpointLocationCollection::ValidForThisThread(Thread &thread) {
   std::lock_guard<std::mutex> guard(m_collection_mutex);
   collection::iterator pos, begin = m_break_loc_collection.begin(),
                             end = m_break_loc_collection.end();
Index: lldb/source/Breakpoint/BreakpointLocation.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointLocation.cpp
+++ lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -52,11 +52,10 @@
   return m_address.GetOpcodeLoadAddress(&m_owner.GetTarget());
 }
 
-const BreakpointOptions *
-BreakpointLocation::GetOptionsSpecifyingKind(BreakpointOptions::OptionKind kind)
-const {
+const BreakpointOptions &BreakpointLocation::GetOptionsSpecifyingKind(
+    BreakpointOptions::OptionKind kind) const {
   if (m_options_up && m_options_up->IsOptionSet(kind))
-    return m_options_up.get();
+    return *m_options_up;
   else
     return m_owner.GetOptions();
 }
@@ -77,7 +76,7 @@
 }
 
 void BreakpointLocation::SetEnabled(bool enabled) {
-  GetLocationOptions()->SetEnabled(enabled);
+  GetLocationOptions().SetEnabled(enabled);
   if (enabled) {
     ResolveBreakpointSite();
   } else {
@@ -96,13 +95,13 @@
 }
 
 void BreakpointLocation::SetAutoContinue(bool auto_continue) {
-  GetLocationOptions()->SetAutoContinue(auto_continue);
+  GetLocationOptions().SetAutoContinue(auto_continue);
   SendBreakpointLocationChangedEvent(eBreakpointEventTypeAutoContinueChanged);
 }
 
 void BreakpointLocation::SetThreadID(lldb::tid_t thread_id) {
   if (thread_id != LLDB_INVALID_THREAD_ID)
-    GetLocationOptions()->SetThreadID(thread_id);
+    GetLocationOptions().SetThreadID(thread_id);
   else {
     // If we're resetting this to an invalid thread id, then don't make an
     // options pointer just to do that.
@@ -113,9 +112,9 @@
 }
 
 lldb::tid_t BreakpointLocation::GetThreadID() {
-  const ThreadSpec *thread_spec = 
+  const ThreadSpec *thread_spec =
       GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
-          ->GetThreadSpecNoCreate();
+          .GetThreadSpecNoCreate();
   if (thread_spec)
     return thread_spec->GetTID();
   else
@@ -124,7 +123,7 @@
 
 void BreakpointLocation::SetThreadIndex(uint32_t index) {
   if (index != 0)
-    GetLocationOptions()->GetThreadSpec()->SetIndex(index);
+    GetLocationOptions().GetThreadSpec()->SetIndex(index);
   else {
     // If we're resetting this to an invalid thread id, then don't make an
     // options pointer just to do that.
@@ -135,9 +134,9 @@
 }
 
 uint32_t BreakpointLocation::GetThreadIndex() const {
-  const ThreadSpec *thread_spec = 
+  const ThreadSpec *thread_spec =
       GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
-          ->GetThreadSpecNoCreate();
+          .GetThreadSpecNoCreate();
   if (thread_spec)
     return thread_spec->GetIndex();
   else
@@ -146,7 +145,7 @@
 
 void BreakpointLocation::SetThreadName(const char *thread_name) {
   if (thread_name != nullptr)
-    GetLocationOptions()->GetThreadSpec()->SetName(thread_name);
+    GetLocationOptions().GetThreadSpec()->SetName(thread_name);
   else {
     // If we're resetting this to an invalid thread id, then don't make an
     // options pointer just to do that.
@@ -157,9 +156,9 @@
 }
 
 const char *BreakpointLocation::GetThreadName() const {
-  const ThreadSpec *thread_spec = 
+  const ThreadSpec *thread_spec =
       GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
-          ->GetThreadSpecNoCreate();
+          .GetThreadSpecNoCreate();
   if (thread_spec)
     return thread_spec->GetName();
   else
@@ -168,7 +167,7 @@
 
 void BreakpointLocation::SetQueueName(const char *queue_name) {
   if (queue_name != nullptr)
-    GetLocationOptions()->GetThreadSpec()->SetQueueName(queue_name);
+    GetLocationOptions().GetThreadSpec()->SetQueueName(queue_name);
   else {
     // If we're resetting this to an invalid thread id, then don't make an
     // options pointer just to do that.
@@ -179,9 +178,9 @@
 }
 
 const char *BreakpointLocation::GetQueueName() const {
-  const ThreadSpec *thread_spec = 
+  const ThreadSpec *thread_spec =
       GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
-          ->GetThreadSpecNoCreate();
+          .GetThreadSpecNoCreate();
   if (thread_spec)
     return thread_spec->GetQueueName();
   else
@@ -199,14 +198,14 @@
   if (m_options_up != nullptr && m_options_up->HasCallback())
     return m_options_up->IsCallbackSynchronous();
   else
-    return m_owner.GetOptions()->IsCallbackSynchronous();
+    return m_owner.GetOptions().IsCallbackSynchronous();
 }
 
 void BreakpointLocation::SetCallback(BreakpointHitCallback callback,
                                      void *baton, bool is_synchronous) {
   // The default "Baton" class will keep a copy of "baton" and won't free or
   // delete it when it goes goes out of scope.
-  GetLocationOptions()->SetCallback(
+  GetLocationOptions().SetCallback(
       callback, std::make_shared<UntypedBaton>(baton), is_synchronous);
   SendBreakpointLocationChangedEvent(eBreakpointEventTypeCommandChanged);
 }
@@ -214,22 +213,22 @@
 void BreakpointLocation::SetCallback(BreakpointHitCallback callback,
                                      const BatonSP &baton_sp,
                                      bool is_synchronous) {
-  GetLocationOptions()->SetCallback(callback, baton_sp, is_synchronous);
+  GetLocationOptions().SetCallback(callback, baton_sp, is_synchronous);
   SendBreakpointLocationChangedEvent(eBreakpointEventTypeCommandChanged);
 }
 
 void BreakpointLocation::ClearCallback() {
-  GetLocationOptions()->ClearCallback();
+  GetLocationOptions().ClearCallback();
 }
 
 void BreakpointLocation::SetCondition(const char *condition) {
-  GetLocationOptions()->SetCondition(condition);
+  GetLocationOptions().SetCondition(condition);
   SendBreakpointLocationChangedEvent(eBreakpointEventTypeConditionChanged);
 }
 
 const char *BreakpointLocation::GetConditionText(size_t *hash) const {
   return GetOptionsSpecifyingKind(BreakpointOptions::eCondition)
-      ->GetConditionText(hash);
+      .GetConditionText(hash);
 }
 
 bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
@@ -340,11 +339,11 @@
 
 uint32_t BreakpointLocation::GetIgnoreCount() const {
   return GetOptionsSpecifyingKind(BreakpointOptions::eIgnoreCount)
-      ->GetIgnoreCount();
+      .GetIgnoreCount();
 }
 
 void BreakpointLocation::SetIgnoreCount(uint32_t n) {
-  GetLocationOptions()->SetIgnoreCount(n);
+  GetLocationOptions().SetIgnoreCount(n);
   SendBreakpointLocationChangedEvent(eBreakpointEventTypeIgnoreChanged);
 }
 
@@ -371,20 +370,20 @@
   return true;
 }
 
-BreakpointOptions *BreakpointLocation::GetLocationOptions() {
+BreakpointOptions &BreakpointLocation::GetLocationOptions() {
   // If we make the copy we don't copy the callbacks because that is
   // potentially expensive and we don't want to do that for the simple case
   // where someone is just disabling the location.
   if (m_options_up == nullptr)
     m_options_up = std::make_unique<BreakpointOptions>(false);
 
-  return m_options_up.get();
+  return *m_options_up;
 }
 
-bool BreakpointLocation::ValidForThisThread(Thread *thread) {
-  return thread
-      ->MatchesSpec(GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
-      ->GetThreadSpecNoCreate());
+bool BreakpointLocation::ValidForThisThread(Thread &thread) {
+  return thread.MatchesSpec(
+      GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
+          .GetThreadSpecNoCreate());
 }
 
 // RETURNS - true if we should stop at this breakpoint, false if we
@@ -631,7 +630,8 @@
       m_bp_site_sp->GetHardwareIndex() : LLDB_INVALID_INDEX32;
 
   lldb::tid_t tid = GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
-      ->GetThreadSpecNoCreate()->GetTID();
+                        .GetThreadSpecNoCreate()
+                        ->GetTID();
   s->Printf("BreakpointLocation %u: tid = %4.4" PRIx64
             "  load addr = 0x%8.8" PRIx64 "  state = %s  type = %s breakpoint  "
             "hw_index = %i  hit_count = %-4u  ignore_count = %-4u",
@@ -640,9 +640,10 @@
             (m_options_up ? m_options_up->IsEnabled() : m_owner.IsEnabled())
                 ? "enabled "
                 : "disabled",
-            is_hardware ? "hardware" : "software", hardware_index, GetHitCount(),
+            is_hardware ? "hardware" : "software", hardware_index,
+            GetHitCount(),
             GetOptionsSpecifyingKind(BreakpointOptions::eIgnoreCount)
-                ->GetIgnoreCount());
+                .GetIgnoreCount());
 }
 
 void BreakpointLocation::SendBreakpointLocationChangedEvent(
Index: lldb/source/Breakpoint/Breakpoint.cpp
===================================================================
--- lldb/source/Breakpoint/Breakpoint.cpp
+++ lldb/source/Breakpoint/Breakpoint.cpp
@@ -49,17 +49,16 @@
                        BreakpointResolverSP &resolver_sp, bool hardware,
                        bool resolve_indirect_symbols)
     : m_being_created(true), m_hardware(hardware), m_target(target),
-      m_filter_sp(filter_sp), m_resolver_sp(resolver_sp),
-      m_options_up(new BreakpointOptions(true)), m_locations(*this),
-      m_resolve_indirect_symbols(resolve_indirect_symbols), m_hit_counter() {
+      m_filter_sp(filter_sp), m_resolver_sp(resolver_sp), m_options(true),
+      m_locations(*this), m_resolve_indirect_symbols(resolve_indirect_symbols),
+      m_hit_counter() {
   m_being_created = false;
 }
 
 Breakpoint::Breakpoint(Target &new_target, const Breakpoint &source_bp)
     : m_being_created(true), m_hardware(source_bp.m_hardware),
       m_target(new_target), m_name_list(source_bp.m_name_list),
-      m_options_up(new BreakpointOptions(*source_bp.m_options_up)),
-      m_locations(*this),
+      m_options(source_bp.m_options), m_locations(*this),
       m_resolve_indirect_symbols(source_bp.m_resolve_indirect_symbols),
       m_hit_counter() {}
 
@@ -116,7 +115,7 @@
                                   filter_dict_sp);
 
   StructuredData::ObjectSP options_dict_sp(
-      m_options_up->SerializeToStructuredData());
+      m_options.SerializeToStructuredData());
   if (!options_dict_sp)
     return StructuredData::ObjectSP();
 
@@ -201,7 +200,7 @@
                                       hardware, true);
 
   if (result_sp && options_up) {
-    result_sp->m_options_up = std::move(options_up);
+    result_sp->m_options = *options_up;
   }
 
   StructuredData::Array *names_array;
@@ -293,10 +292,10 @@
 // individual settings.
 
 void Breakpoint::SetEnabled(bool enable) {
-  if (enable == m_options_up->IsEnabled())
+  if (enable == m_options.IsEnabled())
     return;
 
-  m_options_up->SetEnabled(enable);
+  m_options.SetEnabled(enable);
   if (enable)
     m_locations.ResolveAllBreakpointSites();
   else
@@ -306,111 +305,107 @@
                                     : eBreakpointEventTypeDisabled);
 }
 
-bool Breakpoint::IsEnabled() { return m_options_up->IsEnabled(); }
+bool Breakpoint::IsEnabled() { return m_options.IsEnabled(); }
 
 void Breakpoint::SetIgnoreCount(uint32_t n) {
-  if (m_options_up->GetIgnoreCount() == n)
+  if (m_options.GetIgnoreCount() == n)
     return;
 
-  m_options_up->SetIgnoreCount(n);
+  m_options.SetIgnoreCount(n);
   SendBreakpointChangedEvent(eBreakpointEventTypeIgnoreChanged);
 }
 
 void Breakpoint::DecrementIgnoreCount() {
-  uint32_t ignore = m_options_up->GetIgnoreCount();
+  uint32_t ignore = m_options.GetIgnoreCount();
   if (ignore != 0)
-    m_options_up->SetIgnoreCount(ignore - 1);
+    m_options.SetIgnoreCount(ignore - 1);
 }
 
 uint32_t Breakpoint::GetIgnoreCount() const {
-  return m_options_up->GetIgnoreCount();
+  return m_options.GetIgnoreCount();
 }
 
 uint32_t Breakpoint::GetHitCount() const { return m_hit_counter.GetValue(); }
 
-bool Breakpoint::IsOneShot() const { return m_options_up->IsOneShot(); }
+bool Breakpoint::IsOneShot() const { return m_options.IsOneShot(); }
 
-void Breakpoint::SetOneShot(bool one_shot) {
-  m_options_up->SetOneShot(one_shot);
-}
+void Breakpoint::SetOneShot(bool one_shot) { m_options.SetOneShot(one_shot); }
 
-bool Breakpoint::IsAutoContinue() const { 
-  return m_options_up->IsAutoContinue();
-}
+bool Breakpoint::IsAutoContinue() const { return m_options.IsAutoContinue(); }
 
 void Breakpoint::SetAutoContinue(bool auto_continue) {
-  m_options_up->SetAutoContinue(auto_continue);
+  m_options.SetAutoContinue(auto_continue);
 }
 
 void Breakpoint::SetThreadID(lldb::tid_t thread_id) {
-  if (m_options_up->GetThreadSpec()->GetTID() == thread_id)
+  if (m_options.GetThreadSpec()->GetTID() == thread_id)
     return;
 
-  m_options_up->GetThreadSpec()->SetTID(thread_id);
+  m_options.GetThreadSpec()->SetTID(thread_id);
   SendBreakpointChangedEvent(eBreakpointEventTypeThreadChanged);
 }
 
 lldb::tid_t Breakpoint::GetThreadID() const {
-  if (m_options_up->GetThreadSpecNoCreate() == nullptr)
+  if (m_options.GetThreadSpecNoCreate() == nullptr)
     return LLDB_INVALID_THREAD_ID;
   else
-    return m_options_up->GetThreadSpecNoCreate()->GetTID();
+    return m_options.GetThreadSpecNoCreate()->GetTID();
 }
 
 void Breakpoint::SetThreadIndex(uint32_t index) {
-  if (m_options_up->GetThreadSpec()->GetIndex() == index)
+  if (m_options.GetThreadSpec()->GetIndex() == index)
     return;
 
-  m_options_up->GetThreadSpec()->SetIndex(index);
+  m_options.GetThreadSpec()->SetIndex(index);
   SendBreakpointChangedEvent(eBreakpointEventTypeThreadChanged);
 }
 
 uint32_t Breakpoint::GetThreadIndex() const {
-  if (m_options_up->GetThreadSpecNoCreate() == nullptr)
+  if (m_options.GetThreadSpecNoCreate() == nullptr)
     return 0;
   else
-    return m_options_up->GetThreadSpecNoCreate()->GetIndex();
+    return m_options.GetThreadSpecNoCreate()->GetIndex();
 }
 
 void Breakpoint::SetThreadName(const char *thread_name) {
-  if (m_options_up->GetThreadSpec()->GetName() != nullptr &&
-      ::strcmp(m_options_up->GetThreadSpec()->GetName(), thread_name) == 0)
+  if (m_options.GetThreadSpec()->GetName() != nullptr &&
+      ::strcmp(m_options.GetThreadSpec()->GetName(), thread_name) == 0)
     return;
 
-  m_options_up->GetThreadSpec()->SetName(thread_name);
+  m_options.GetThreadSpec()->SetName(thread_name);
   SendBreakpointChangedEvent(eBreakpointEventTypeThreadChanged);
 }
 
 const char *Breakpoint::GetThreadName() const {
-  if (m_options_up->GetThreadSpecNoCreate() == nullptr)
+  if (m_options.GetThreadSpecNoCreate() == nullptr)
     return nullptr;
   else
-    return m_options_up->GetThreadSpecNoCreate()->GetName();
+    return m_options.GetThreadSpecNoCreate()->GetName();
 }
 
 void Breakpoint::SetQueueName(const char *queue_name) {
-  if (m_options_up->GetThreadSpec()->GetQueueName() != nullptr &&
-      ::strcmp(m_options_up->GetThreadSpec()->GetQueueName(), queue_name) == 0)
+  if (m_options.GetThreadSpec()->GetQueueName() != nullptr &&
+      ::strcmp(m_options.GetThreadSpec()->GetQueueName(), queue_name) == 0)
     return;
 
-  m_options_up->GetThreadSpec()->SetQueueName(queue_name);
+  m_options.GetThreadSpec()->SetQueueName(queue_name);
   SendBreakpointChangedEvent(eBreakpointEventTypeThreadChanged);
 }
 
 const char *Breakpoint::GetQueueName() const {
-  if (m_options_up->GetThreadSpecNoCreate() == nullptr)
+  if (m_options.GetThreadSpecNoCreate() == nullptr)
     return nullptr;
   else
-    return m_options_up->GetThreadSpecNoCreate()->GetQueueName();
+    return m_options.GetThreadSpecNoCreate()->GetQueueName();
 }
 
 void Breakpoint::SetCondition(const char *condition) {
-  m_options_up->SetCondition(condition);
+  m_options.SetCondition(condition);
   SendBreakpointChangedEvent(eBreakpointEventTypeConditionChanged);
 }
 
 const char *Breakpoint::GetConditionText() const {
-  return m_options_up->GetConditionText();
+  return m_options.GetConditionText();
 }
 
 // This function is used when "baton" doesn't need to be freed
@@ -418,8 +413,8 @@
                              bool is_synchronous) {
   // The default "Baton" class will keep a copy of "baton" and won't free or
   // delete it when it goes goes out of scope.
-  m_options_up->SetCallback(callback, std::make_shared<UntypedBaton>(baton),
-                            is_synchronous);
+  m_options.SetCallback(callback, std::make_shared<UntypedBaton>(baton),
+                        is_synchronous);
 
   SendBreakpointChangedEvent(eBreakpointEventTypeCommandChanged);
 }
@@ -429,21 +424,19 @@
 void Breakpoint::SetCallback(BreakpointHitCallback callback,
                              const BatonSP &callback_baton_sp,
                              bool is_synchronous) {
-  m_options_up->SetCallback(callback, callback_baton_sp, is_synchronous);
+  m_options.SetCallback(callback, callback_baton_sp, is_synchronous);
 }
 
-void Breakpoint::ClearCallback() { m_options_up->ClearCallback(); }
+void Breakpoint::ClearCallback() { m_options.ClearCallback(); }
 
 bool Breakpoint::InvokeCallback(StoppointCallbackContext *context,
                                 break_id_t bp_loc_id) {
-  return m_options_up->InvokeCallback(context, GetID(), bp_loc_id);
+  return m_options.InvokeCallback(context, GetID(), bp_loc_id);
 }
 
-BreakpointOptions *Breakpoint::GetOptions() { return m_options_up.get(); }
+BreakpointOptions &Breakpoint::GetOptions() { return m_options; }
 
-const BreakpointOptions *Breakpoint::GetOptions() const {
-  return m_options_up.get();
-}
+const BreakpointOptions &Breakpoint::GetOptions() const { return m_options; }
 
 void Breakpoint::ResolveBreakpoint() {
   if (m_resolver_sp)
@@ -890,7 +883,7 @@
         s->Printf(", locations = 0 (pending)");
     }
 
-    GetOptions()->GetDescription(s, level);
+    m_options.GetDescription(s, level);
 
     if (m_precondition_sp)
       m_precondition_sp->GetDescription(*s, level);
@@ -932,7 +925,7 @@
     Dump(s);
     s->EOL();
     // s->Indent();
-    GetOptions()->GetDescription(s, level);
+    m_options.GetDescription(s, level);
     break;
 
   default:
Index: lldb/source/API/SBBreakpointName.cpp
===================================================================
--- lldb/source/API/SBBreakpointName.cpp
+++ lldb/source/API/SBBreakpointName.cpp
@@ -144,7 +144,7 @@
   }
 
   // Now copy over the breakpoint's options:
-  target.ConfigureBreakpointName(*bp_name, *bkpt_sp->GetOptions(),
+  target.ConfigureBreakpointName(*bp_name, bkpt_sp->GetOptions(),
                                  BreakpointName::Permissions());
 }
 
@@ -594,12 +594,11 @@
   BreakpointOptions &bp_options = bp_name->GetOptions();
   Status error;
   error = m_impl_up->GetTarget()
-      ->GetDebugger()
-      .GetScriptInterpreter()
-      ->SetBreakpointCommandCallbackFunction(&bp_options,
-                                             callback_function_name,
-                                             extra_args.m_impl_up
-                                                 ->GetObjectSP());
+              ->GetDebugger()
+              .GetScriptInterpreter()
+              ->SetBreakpointCommandCallbackFunction(
+                  bp_options, callback_function_name,
+                  extra_args.m_impl_up->GetObjectSP());
   sb_error.SetError(error);
   UpdateName(*bp_name);
   return LLDB_RECORD_RESULT(sb_error);
@@ -623,7 +622,7 @@
       m_impl_up->GetTarget()
           ->GetDebugger()
           .GetScriptInterpreter()
-          ->SetBreakpointCommandCallback(&bp_options, callback_body_text);
+          ->SetBreakpointCommandCallback(bp_options, callback_body_text);
   sb_error.SetError(error);
   if (!sb_error.Fail())
     UpdateName(*bp_name);
Index: lldb/source/API/SBBreakpointLocation.cpp
===================================================================
--- lldb/source/API/SBBreakpointLocation.cpp
+++ lldb/source/API/SBBreakpointLocation.cpp
@@ -227,7 +227,7 @@
     Status error;
     std::lock_guard<std::recursive_mutex> guard(
         loc_sp->GetTarget().GetAPIMutex());
-    BreakpointOptions *bp_options = loc_sp->GetLocationOptions();
+    BreakpointOptions &bp_options = loc_sp->GetLocationOptions();
     error = loc_sp->GetBreakpoint()
         .GetTarget()
         .GetDebugger()
@@ -254,7 +254,7 @@
   if (loc_sp) {
     std::lock_guard<std::recursive_mutex> guard(
         loc_sp->GetTarget().GetAPIMutex());
-    BreakpointOptions *bp_options = loc_sp->GetLocationOptions();
+    BreakpointOptions &bp_options = loc_sp->GetLocationOptions();
     Status error =
         loc_sp->GetBreakpoint()
             .GetTarget()
@@ -283,7 +283,7 @@
   std::unique_ptr<BreakpointOptions::CommandData> cmd_data_up(
       new BreakpointOptions::CommandData(*commands, eScriptLanguageNone));
 
-  loc_sp->GetLocationOptions()->SetCommandDataCallback(cmd_data_up);
+  loc_sp->GetLocationOptions().SetCommandDataCallback(cmd_data_up);
 }
 
 bool SBBreakpointLocation::GetCommandLineCommands(SBStringList &commands) {
@@ -295,7 +295,7 @@
     return false;
   StringList command_list;
   bool has_commands =
-      loc_sp->GetLocationOptions()->GetCommandLineCallbacks(command_list);
+      loc_sp->GetLocationOptions().GetCommandLineCallbacks(command_list);
   if (has_commands)
     commands.AppendList(command_list);
   return has_commands;
Index: lldb/source/API/SBBreakpoint.cpp
===================================================================
--- lldb/source/API/SBBreakpoint.cpp
+++ lldb/source/API/SBBreakpoint.cpp
@@ -384,7 +384,7 @@
   if (bkpt_sp) {
     std::lock_guard<std::recursive_mutex> guard(
         bkpt_sp->GetTarget().GetAPIMutex());
-    bkpt_sp->GetOptions()->GetThreadSpec()->SetIndex(index);
+    bkpt_sp->GetOptions().GetThreadSpec()->SetIndex(index);
   }
 }
 
@@ -397,7 +397,7 @@
     std::lock_guard<std::recursive_mutex> guard(
         bkpt_sp->GetTarget().GetAPIMutex());
     const ThreadSpec *thread_spec =
-        bkpt_sp->GetOptions()->GetThreadSpecNoCreate();
+        bkpt_sp->GetOptions().GetThreadSpecNoCreate();
     if (thread_spec != nullptr)
       thread_idx = thread_spec->GetIndex();
   }
@@ -414,7 +414,7 @@
   if (bkpt_sp) {
     std::lock_guard<std::recursive_mutex> guard(
         bkpt_sp->GetTarget().GetAPIMutex());
-    bkpt_sp->GetOptions()->GetThreadSpec()->SetName(thread_name);
+    bkpt_sp->GetOptions().GetThreadSpec()->SetName(thread_name);
   }
 }
 
@@ -427,7 +427,7 @@
     std::lock_guard<std::recursive_mutex> guard(
         bkpt_sp->GetTarget().GetAPIMutex());
     const ThreadSpec *thread_spec =
-        bkpt_sp->GetOptions()->GetThreadSpecNoCreate();
+        bkpt_sp->GetOptions().GetThreadSpecNoCreate();
     if (thread_spec != nullptr)
       name = thread_spec->GetName();
   }
@@ -443,7 +443,7 @@
   if (bkpt_sp) {
     std::lock_guard<std::recursive_mutex> guard(
         bkpt_sp->GetTarget().GetAPIMutex());
-    bkpt_sp->GetOptions()->GetThreadSpec()->SetQueueName(queue_name);
+    bkpt_sp->GetOptions().GetThreadSpec()->SetQueueName(queue_name);
   }
 }
 
@@ -456,7 +456,7 @@
     std::lock_guard<std::recursive_mutex> guard(
         bkpt_sp->GetTarget().GetAPIMutex());
     const ThreadSpec *thread_spec =
-        bkpt_sp->GetOptions()->GetThreadSpecNoCreate();
+        bkpt_sp->GetOptions().GetThreadSpecNoCreate();
     if (thread_spec)
       name = thread_spec->GetQueueName();
   }
@@ -506,7 +506,7 @@
   std::unique_ptr<BreakpointOptions::CommandData> cmd_data_up(
       new BreakpointOptions::CommandData(*commands, eScriptLanguageNone));
 
-  bkpt_sp->GetOptions()->SetCommandDataCallback(cmd_data_up);
+  bkpt_sp->GetOptions().SetCommandDataCallback(cmd_data_up);
 }
 
 bool SBBreakpoint::GetCommandLineCommands(SBStringList &commands) {
@@ -518,7 +518,7 @@
     return false;
   StringList command_list;
   bool has_commands =
-      bkpt_sp->GetOptions()->GetCommandLineCallbacks(command_list);
+      bkpt_sp->GetOptions().GetCommandLineCallbacks(command_list);
   if (has_commands)
     commands.AppendList(command_list);
   return has_commands;
@@ -636,7 +636,7 @@
     Status error;
     std::lock_guard<std::recursive_mutex> guard(
         bkpt_sp->GetTarget().GetAPIMutex());
-    BreakpointOptions *bp_options = bkpt_sp->GetOptions();
+    BreakpointOptions &bp_options = bkpt_sp->GetOptions();
     error = bkpt_sp->GetTarget()
         .GetDebugger()
         .GetScriptInterpreter()
@@ -661,7 +661,7 @@
   if (bkpt_sp) {
     std::lock_guard<std::recursive_mutex> guard(
         bkpt_sp->GetTarget().GetAPIMutex());
-    BreakpointOptions *bp_options = bkpt_sp->GetOptions();
+    BreakpointOptions &bp_options = bkpt_sp->GetOptions();
     Status error =
         bkpt_sp->GetTarget()
             .GetDebugger()
Index: lldb/include/lldb/Interpreter/ScriptInterpreter.h
===================================================================
--- lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -338,18 +338,19 @@
   }
 
   virtual void CollectDataForBreakpointCommandCallback(
-      std::vector<BreakpointOptions *> &options, CommandReturnObject &result);
+      std::vector<std::reference_wrapper<BreakpointOptions>> &options,
+      CommandReturnObject &result);
 
   virtual void
   CollectDataForWatchpointCommandCallback(WatchpointOptions *wp_options,
                                           CommandReturnObject &result);
 
   /// Set the specified text as the callback for the breakpoint.
-  Status
-  SetBreakpointCommandCallback(std::vector<BreakpointOptions *> &bp_options_vec,
-                               const char *callback_text);
+  Status SetBreakpointCommandCallback(
+      std::vector<std::reference_wrapper<BreakpointOptions>> &bp_options_vec,
+      const char *callback_text);
 
-  virtual Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  virtual Status SetBreakpointCommandCallback(BreakpointOptions &bp_options,
                                               const char *callback_text) {
     Status error;
     error.SetErrorString("unimplemented");
@@ -358,7 +359,7 @@
 
   /// This one is for deserialization:
   virtual Status SetBreakpointCommandCallback(
-      BreakpointOptions *bp_options,
+      BreakpointOptions &bp_options,
       std::unique_ptr<BreakpointOptions::CommandData> &data_up) {
     Status error;
     error.SetErrorString("unimplemented");
@@ -366,15 +367,14 @@
   }
 
   Status SetBreakpointCommandCallbackFunction(
-      std::vector<BreakpointOptions *> &bp_options_vec,
+      std::vector<std::reference_wrapper<BreakpointOptions>> &bp_options_vec,
       const char *function_name, StructuredData::ObjectSP extra_args_sp);
 
   /// Set a script function as the callback for the breakpoint.
   virtual Status
-  SetBreakpointCommandCallbackFunction(
-      BreakpointOptions *bp_options,
-      const char *function_name,
-      StructuredData::ObjectSP extra_args_sp) {
+  SetBreakpointCommandCallbackFunction(BreakpointOptions &bp_options,
+                                       const char *function_name,
+                                       StructuredData::ObjectSP extra_args_sp) {
     Status error;
     error.SetErrorString("unimplemented");
     return error;
Index: lldb/include/lldb/Breakpoint/BreakpointSite.h
===================================================================
--- lldb/include/lldb/Breakpoint/BreakpointSite.h
+++ lldb/include/lldb/Breakpoint/BreakpointSite.h
@@ -148,7 +148,7 @@
   /// return
   ///     \b true if the collection contains at least one location that
   ///     would be valid for this thread, false otherwise.
-  bool ValidForThisThread(Thread *thread);
+  bool ValidForThisThread(Thread &thread);
 
   /// Print a description of this breakpoint site to the stream \a s.
   /// GetDescription tells you about the breakpoint site's owners. Use
Index: lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h
===================================================================
--- lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h
+++ lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h
@@ -136,7 +136,7 @@
   /// return
   ///     \b true if the collection contains at least one location that
   ///     would be valid for this thread, false otherwise.
-  bool ValidForThisThread(Thread *thread);
+  bool ValidForThisThread(Thread &thread);
 
   /// Tell whether ALL the breakpoints in the location collection are internal.
   ///
Index: lldb/include/lldb/Breakpoint/BreakpointLocation.h
===================================================================
--- lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -202,8 +202,8 @@
   /// hasn't been done already
   ///
   /// \return
-  ///    A pointer to the breakpoint options.
-  BreakpointOptions *GetLocationOptions();
+  ///    A reference to the breakpoint options.
+  BreakpointOptions &GetLocationOptions();
 
   /// Use this to access breakpoint options from this breakpoint location.
   /// This will return the options that have a setting for the specified
@@ -214,10 +214,10 @@
   /// \return
   ///     A pointer to the containing breakpoint's options if this
   ///     location doesn't have its own copy.
-  const BreakpointOptions *GetOptionsSpecifyingKind(
-      BreakpointOptions::OptionKind kind) const;
+  const BreakpointOptions &
+  GetOptionsSpecifyingKind(BreakpointOptions::OptionKind kind) const;
 
-  bool ValidForThisThread(Thread *thread);
+  bool ValidForThisThread(Thread &thread);
 
   /// Invoke the callback action when the breakpoint is hit.
   ///
Index: lldb/include/lldb/Breakpoint/Breakpoint.h
===================================================================
--- lldb/include/lldb/Breakpoint/Breakpoint.h
+++ lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -481,16 +481,16 @@
   /// Meant to be used by the BreakpointLocation class.
   ///
   /// \return
-  ///     A pointer to this breakpoint's BreakpointOptions.
-  BreakpointOptions *GetOptions();
+  ///     A reference to this breakpoint's BreakpointOptions.
+  BreakpointOptions &GetOptions();
 
   /// Returns the BreakpointOptions structure set at the breakpoint level.
   ///
   /// Meant to be used by the BreakpointLocation class.
   ///
   /// \return
-  ///     A pointer to this breakpoint's BreakpointOptions.
-  const BreakpointOptions *GetOptions() const;
+  ///     A reference to this breakpoint's BreakpointOptions.
+  const BreakpointOptions &GetOptions() const;
 
   /// Invoke the callback action when the breakpoint is hit.
   ///
@@ -640,8 +640,7 @@
   // to skip certain breakpoint hits.  For instance, exception breakpoints use
   // this to limit the stop to certain exception classes, while leaving the
   // condition & callback free for user specification.
-  std::unique_ptr<BreakpointOptions>
-      m_options_up; // Settable breakpoint options
+  BreakpointOptions m_options; // Settable breakpoint options
   BreakpointLocationList
       m_locations; // The list of locations currently found for this breakpoint.
   std::string m_kind_description;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] D104162... Jim Ingham via Phabricator via lldb-commits

Reply via email to