Convert functions that were returning BreakpointOption * to BreakpointOption &.

This is an NFC cleanup.

Many of the API's that returned BreakpointOptions always returned valid ones.
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.

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.

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

1 files changed, 36 insertions, 35 deletions

diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index acdffa12369c..0c4d3e052c53 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -52,11 +52,10 @@ lldb::addr_t BreakpointLocation::GetLoadAddress() const {
   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 @@ bool BreakpointLocation::IsEnabled() const {
 }
 
 void BreakpointLocation::SetEnabled(bool enabled) {
-  GetLocationOptions()->SetEnabled(enabled);
+  GetLocationOptions().SetEnabled(enabled);
   if (enabled) {
     ResolveBreakpointSite();
   } else {
@@ -96,13 +95,13 @@ bool BreakpointLocation::IsAutoContinue() const {
 }
 
 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 @@ void BreakpointLocation::SetThreadID(lldb::tid_t thread_id) {
 }
 
 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 @@ lldb::tid_t BreakpointLocation::GetThreadID() {
 
 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 @@ void BreakpointLocation::SetThreadIndex(uint32_t index) {
 }
 
 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 @@ uint32_t BreakpointLocation::GetThreadIndex() const {
 
 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 @@ void BreakpointLocation::SetThreadName(const char *thread_name) {
 }
 
 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 @@ const char *BreakpointLocation::GetThreadName() const {
 
 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 @@ void BreakpointLocation::SetQueueName(const char *queue_name) {
 }
 
 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 @@ bool BreakpointLocation::IsCallbackSynchronous() {
   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,
 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 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
 
 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 @@ bool BreakpointLocation::IgnoreCountShouldStop() {
   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 @@ void BreakpointLocation::Dump(Stream *s) const {
       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 @@ void BreakpointLocation::Dump(Stream *s) const {
             (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(