[SBAPI] Don't check IsValid in constructor

When running the test suite with the instrumentation macros, I noticed
two lldb-mi tests regressed. The issue was the copy constructor of
SBLineEntry. Without the macros the returned value would be elided, but
with the macros the copy constructor was called. The latter using ::IsValid
to determine whether the underlying opaque pointer should be set. This
is likely a remnant of when ::IsValid would only check the validity of the
smart pointer. In SBLineEntry however, it actually forwards to
LineEntry::IsValid().

So what happened here was that because of the macros the copy
constructor was called. The opaque pointer was valid but the LineEntry
didn't consider itself valid. So the copied-to object ended up default
initialized.

This patch replaces all checks for IsValid in copy (assignment)
constructors with checks for the opaque pointer itself.

Differential revision: https://reviews.llvm.org/D58946

llvm-svn: 355458

1 files changed, 9 insertions, 20 deletions

diff --git a/lldb/source/API/SBSymbolContext.cpp b/lldb/source/API/SBSymbolContext.cpp
index 0260d1dc0db5..ae426709f180 100644
--- a/lldb/source/API/SBSymbolContext.cpp
+++ b/lldb/source/API/SBSymbolContext.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/API/SBSymbolContext.h"
+#include "Utils.h"
 #include "lldb/API/SBStream.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/Function.h"
@@ -21,38 +22,26 @@ SBSymbolContext::SBSymbolContext() : m_opaque_up() {}
 
 SBSymbolContext::SBSymbolContext(const SymbolContext *sc_ptr) : m_opaque_up() {
   if (sc_ptr)
-    m_opaque_up.reset(new SymbolContext(*sc_ptr));
+    m_opaque_up = llvm::make_unique<SymbolContext>(*sc_ptr);
 }
 
 SBSymbolContext::SBSymbolContext(const SBSymbolContext &rhs) : m_opaque_up() {
-  if (rhs.IsValid()) {
-    if (m_opaque_up)
-      *m_opaque_up = *rhs.m_opaque_up;
-    else
-      ref() = *rhs.m_opaque_up;
-  }
+  m_opaque_up = clone(rhs.m_opaque_up);
 }
 
 SBSymbolContext::~SBSymbolContext() {}
 
 const SBSymbolContext &SBSymbolContext::operator=(const SBSymbolContext &rhs) {
-  if (this != &rhs) {
-    if (rhs.IsValid())
-      m_opaque_up.reset(new lldb_private::SymbolContext(*rhs.m_opaque_up));
-  }
+  if (this != &rhs)
+    m_opaque_up = clone(rhs.m_opaque_up);
   return *this;
 }
 
 void SBSymbolContext::SetSymbolContext(const SymbolContext *sc_ptr) {
-  if (sc_ptr) {
-    if (m_opaque_up)
-      *m_opaque_up = *sc_ptr;
-    else
-      m_opaque_up.reset(new SymbolContext(*sc_ptr));
-  } else {
-    if (m_opaque_up)
-      m_opaque_up->Clear(true);
-  }
+  if (sc_ptr)
+    m_opaque_up = llvm::make_unique<SymbolContext>(*sc_ptr);
+  else
+    m_opaque_up->Clear(true);
 }
 
 bool SBSymbolContext::IsValid() const { return m_opaque_up != NULL; }