From 2ef442c639dcf6211bb779402cf6bbe102e2fbaa Mon Sep 17 00:00:00 2001 From: Todd Fiala Date: Wed, 9 Nov 2016 23:21:04 +0000 Subject: [PATCH] Fix weak symbol linkage in SBStructuredData, update docs. Summary: This change fixes an issue where I was leaking a weakly-linked symbol in the SBAPI. It also updates the docs to call out what I did wrong. Fixes: rdar://28882483 Reviewers: jingham Subscribers: lldb-commits Differential Revision: https://reviews.llvm.org/D26470 llvm-svn: 286413 --- lldb/include/lldb/API/SBStructuredData.h | 5 ++- lldb/source/API/SBStructuredData.cpp | 53 ++++++++++++------------ lldb/www/SB-api-coding-rules.html | 7 ++-- 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/lldb/include/lldb/API/SBStructuredData.h b/lldb/include/lldb/API/SBStructuredData.h index 1be93f331ecd..9f0203bb245a 100644 --- a/lldb/include/lldb/API/SBStructuredData.h +++ b/lldb/include/lldb/API/SBStructuredData.h @@ -13,6 +13,8 @@ #include "lldb/API/SBDefines.h" #include "lldb/API/SBModule.h" +class StructuredDataImpl; + namespace lldb { class SBStructuredData { @@ -36,8 +38,7 @@ public: lldb::SBError GetDescription(lldb::SBStream &stream) const; private: - class Impl; - std::unique_ptr m_impl_up; + std::unique_ptr m_impl_up; }; } diff --git a/lldb/source/API/SBStructuredData.cpp b/lldb/source/API/SBStructuredData.cpp index 3c04b3bbba96..d9ea072186a9 100644 --- a/lldb/source/API/SBStructuredData.cpp +++ b/lldb/source/API/SBStructuredData.cpp @@ -20,23 +20,23 @@ using namespace lldb; using namespace lldb_private; #pragma mark-- -#pragma mark Impl +#pragma mark StructuredDataImpl -class SBStructuredData::Impl { +class StructuredDataImpl { public: - Impl() : m_plugin_wp(), m_data_sp() {} + StructuredDataImpl() : m_plugin_wp(), m_data_sp() {} - Impl(const Impl &rhs) = default; + StructuredDataImpl(const StructuredDataImpl &rhs) = default; - Impl(const EventSP &event_sp) + StructuredDataImpl(const EventSP &event_sp) : m_plugin_wp( EventDataStructuredData::GetPluginFromEvent(event_sp.get())), m_data_sp(EventDataStructuredData::GetObjectFromEvent(event_sp.get())) { } - ~Impl() = default; + ~StructuredDataImpl() = default; - Impl &operator=(const Impl &rhs) = default; + StructuredDataImpl &operator=(const StructuredDataImpl &rhs) = default; bool IsValid() const { return m_data_sp.get() != nullptr; } @@ -45,7 +45,7 @@ public: m_data_sp.reset(); } - SBError GetAsJSON(lldb::SBStream &stream) const { + SBError GetAsJSON(lldb_private::Stream &stream) const { SBError sb_error; if (!m_data_sp) { @@ -53,33 +53,29 @@ public: return sb_error; } - m_data_sp->Dump(stream.ref()); + m_data_sp->Dump(stream); return sb_error; } - lldb::SBError GetDescription(lldb::SBStream &stream) const { - SBError sb_error; + Error GetDescription(lldb_private::Stream &stream) const { + Error error; if (!m_data_sp) { - sb_error.SetErrorString("Cannot pretty print structured data: " - "no data to print."); - return sb_error; + error.SetErrorString("Cannot pretty print structured data: " + "no data to print."); + return error; } // Grab the plugin. auto plugin_sp = StructuredDataPluginSP(m_plugin_wp); if (!plugin_sp) { - sb_error.SetErrorString("Cannot pretty print structured data: " - "plugin doesn't exist."); - return sb_error; + error.SetErrorString("Cannot pretty print structured data: " + "plugin doesn't exist."); + return error; } // Get the data's description. - auto error = plugin_sp->GetDescription(m_data_sp, stream.ref()); - if (!error.Success()) - sb_error.SetError(error); - - return sb_error; + return plugin_sp->GetDescription(m_data_sp, stream); } private: @@ -90,13 +86,13 @@ private: #pragma mark-- #pragma mark SBStructuredData -SBStructuredData::SBStructuredData() : m_impl_up(new Impl()) {} +SBStructuredData::SBStructuredData() : m_impl_up(new StructuredDataImpl()) {} SBStructuredData::SBStructuredData(const lldb::SBStructuredData &rhs) - : m_impl_up(new Impl(*rhs.m_impl_up.get())) {} + : m_impl_up(new StructuredDataImpl(*rhs.m_impl_up.get())) {} SBStructuredData::SBStructuredData(const lldb::EventSP &event_sp) - : m_impl_up(new Impl(event_sp)) {} + : m_impl_up(new StructuredDataImpl(event_sp)) {} SBStructuredData::~SBStructuredData() {} @@ -111,9 +107,12 @@ bool SBStructuredData::IsValid() const { return m_impl_up->IsValid(); } void SBStructuredData::Clear() { m_impl_up->Clear(); } SBError SBStructuredData::GetAsJSON(lldb::SBStream &stream) const { - return m_impl_up->GetAsJSON(stream); + return m_impl_up->GetAsJSON(stream.ref()); } lldb::SBError SBStructuredData::GetDescription(lldb::SBStream &stream) const { - return m_impl_up->GetDescription(stream); + Error error = m_impl_up->GetDescription(stream.ref()); + SBError sb_error; + sb_error.SetError(error); + return sb_error; } diff --git a/lldb/www/SB-api-coding-rules.html b/lldb/www/SB-api-coding-rules.html index 0f7ef0e1eb5b..83783b3a86a3 100644 --- a/lldb/www/SB-api-coding-rules.html +++ b/lldb/www/SB-api-coding-rules.html @@ -36,7 +36,7 @@

You also need to choose the ivars for the class with care, since you can't add or remove ivars without breaking binary compatibility. In some cases, the SB class is a thin wrapper around - an interal lldb_private object. In that case, the class can have a single ivar, which is + an internal lldb_private object. In that case, the class can have a single ivar, which is either a pointer, shared_ptr or unique_ptr to the object in the lldb_private API. All the lldb_private classes that get used this way are declared as opaque classes in lldb_forward.h, which is included in SBDefines.h. So if you need an SB class to wrap an lldb_private class @@ -47,8 +47,9 @@ as a direct ivar in the SB Class. Instead, make an Impl class in the SB's .cpp file, and then make the SB object hold a shared or unique pointer to the Impl object. The theory behind this is that if you need more state in the SB object, those needs are likely to change over time, - and this way the impl class can pick up members without changing the size of the object. - An example of this is the SBValue class. + and this way the Impl class can pick up members without changing the size of the object. + An example of this is the SBValue class. Please note that you should not put this Impl class + in the lldb namespace. Failure to do so leads to leakage of weak-linked symbols in the SBAPI.

In order to fit into the Python API's, we need to be able to default construct all the SB objects. Since the ivars of the classes are all pointers of one sort or other, this can easily be done, but