Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lldb][NFC] Move some ctors and tors to cpp files #67165

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

walter-erquinigo
Copy link
Member

This prevents undefined vtable errors when linking these libraries from out-of-tree. I'm facing this issue as I work on my new language plugin.

@walter-erquinigo walter-erquinigo marked this pull request as ready for review September 22, 2023 16:48
@llvmbot llvmbot added the lldb label Sep 22, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 22, 2023

@llvm/pr-subscribers-lldb

Changes

This prevents undefined vtable errors when linking these libraries from out-of-tree. I'm facing this issue as I work on my new language plugin.


Full diff: https://github.com/llvm/llvm-project/pull/67165.diff

4 Files Affected:

  • (modified) lldb/include/lldb/Core/UserSettingsController.h (+3-4)
  • (modified) lldb/include/lldb/Symbol/Function.h (+6-16)
  • (modified) lldb/source/Core/UserSettingsController.cpp (+7)
  • (modified) lldb/source/Symbol/Function.cpp (+25)
diff --git a/lldb/include/lldb/Core/UserSettingsController.h b/lldb/include/lldb/Core/UserSettingsController.h
index ea60467c9afe5c0..32da7e05f7040f7 100644
--- a/lldb/include/lldb/Core/UserSettingsController.h
+++ b/lldb/include/lldb/Core/UserSettingsController.h
@@ -32,12 +32,11 @@ namespace lldb_private {
 
 class Properties {
 public:
-  Properties() = default;
+  Properties();
 
-  Properties(const lldb::OptionValuePropertiesSP &collection_sp)
-      : m_collection_sp(collection_sp) {}
+  Properties(const lldb::OptionValuePropertiesSP &collection_sp);
 
-  virtual ~Properties() = default;
+  virtual ~Properties();
 
   virtual lldb::OptionValuePropertiesSP GetValueProperties() const {
     // This function is virtual in case subclasses want to lazily implement
diff --git a/lldb/include/lldb/Symbol/Function.h b/lldb/include/lldb/Symbol/Function.h
index 2da13f878a7992c..0c1afd0ceb6f1f5 100644
--- a/lldb/include/lldb/Symbol/Function.h
+++ b/lldb/include/lldb/Symbol/Function.h
@@ -267,7 +267,7 @@ using CallSiteParameterArray = llvm::SmallVector<CallSiteParameter, 0>;
 class CallEdge {
 public:
   enum class AddrType : uint8_t { Call, AfterCall };
-  virtual ~CallEdge() = default;
+  ~CallEdge();
 
   /// Get the callee's definition.
   ///
@@ -305,10 +305,7 @@ class CallEdge {
 
 protected:
   CallEdge(AddrType caller_address_type, lldb::addr_t caller_address,
-           bool is_tail_call, CallSiteParameterArray &&parameters)
-      : caller_address(caller_address),
-        caller_address_type(caller_address_type), is_tail_call(is_tail_call),
-        parameters(std::move(parameters)) {}
+           bool is_tail_call, CallSiteParameterArray &&parameters);
 
   /// Helper that finds the load address of \p unresolved_pc, a file address
   /// which refers to an instruction within \p caller.
@@ -339,11 +336,7 @@ class DirectCallEdge : public CallEdge {
   /// return PC within the calling function to identify a specific call site.
   DirectCallEdge(const char *symbol_name, AddrType caller_address_type,
                  lldb::addr_t caller_address, bool is_tail_call,
-                 CallSiteParameterArray &&parameters)
-      : CallEdge(caller_address_type, caller_address, is_tail_call,
-                 std::move(parameters)) {
-    lazy_callee.symbol_name = symbol_name;
-  }
+                 CallSiteParameterArray &&parameters);
 
   Function *GetCallee(ModuleList &images, ExecutionContext &exe_ctx) override;
 
@@ -370,12 +363,9 @@ class IndirectCallEdge : public CallEdge {
 public:
   /// Construct a call edge using a DWARFExpression to identify the callee, and
   /// a return PC within the calling function to identify a specific call site.
-  IndirectCallEdge(DWARFExpressionList call_target, AddrType caller_address_type,
-                   lldb::addr_t caller_address, bool is_tail_call,
-                   CallSiteParameterArray &&parameters)
-      : CallEdge(caller_address_type, caller_address, is_tail_call,
-                 std::move(parameters)),
-        call_target(std::move(call_target)) {}
+  IndirectCallEdge(DWARFExpressionList call_target,
+                   AddrType caller_address_type, lldb::addr_t caller_address,
+                   bool is_tail_call, CallSiteParameterArray &&parameters);
 
   Function *GetCallee(ModuleList &images, ExecutionContext &exe_ctx) override;
 
diff --git a/lldb/source/Core/UserSettingsController.cpp b/lldb/source/Core/UserSettingsController.cpp
index f5dd926cf0500b9..72217117557ffdc 100644
--- a/lldb/source/Core/UserSettingsController.cpp
+++ b/lldb/source/Core/UserSettingsController.cpp
@@ -30,6 +30,13 @@ class Property;
 using namespace lldb;
 using namespace lldb_private;
 
+Properties::Properties() = default;
+
+Properties::Properties(const lldb::OptionValuePropertiesSP &collection_sp)
+    : m_collection_sp(collection_sp) {}
+
+Properties::~Properties() = default;
+
 lldb::OptionValueSP
 Properties::GetPropertyValue(const ExecutionContext *exe_ctx,
                              llvm::StringRef path, Status &error) const {
diff --git a/lldb/source/Symbol/Function.cpp b/lldb/source/Symbol/Function.cpp
index 5ed0e66bdacbd57..c651e4573854108 100644
--- a/lldb/source/Symbol/Function.cpp
+++ b/lldb/source/Symbol/Function.cpp
@@ -122,6 +122,13 @@ size_t InlineFunctionInfo::MemorySize() const {
 /// @name Call site related structures
 /// @{
 
+CallEdge::~CallEdge() = default;
+
+CallEdge::CallEdge(AddrType caller_address_type, lldb::addr_t caller_address,
+                   bool is_tail_call, CallSiteParameterArray &&parameters)
+    : caller_address(caller_address), caller_address_type(caller_address_type),
+      is_tail_call(is_tail_call), parameters(std::move(parameters)) {}
+
 lldb::addr_t CallEdge::GetLoadAddress(lldb::addr_t unresolved_pc,
                                       Function &caller, Target &target) {
   Log *log = GetLog(LLDBLog::Step);
@@ -185,12 +192,30 @@ void DirectCallEdge::ParseSymbolFileAndResolve(ModuleList &images) {
   resolved = true;
 }
 
+DirectCallEdge::DirectCallEdge(const char *symbol_name,
+                               AddrType caller_address_type,
+                               lldb::addr_t caller_address, bool is_tail_call,
+                               CallSiteParameterArray &&parameters)
+    : CallEdge(caller_address_type, caller_address, is_tail_call,
+               std::move(parameters)) {
+  lazy_callee.symbol_name = symbol_name;
+}
+
 Function *DirectCallEdge::GetCallee(ModuleList &images, ExecutionContext &) {
   ParseSymbolFileAndResolve(images);
   assert(resolved && "Did not resolve lazy callee");
   return lazy_callee.def;
 }
 
+IndirectCallEdge::IndirectCallEdge(DWARFExpressionList call_target,
+                                   AddrType caller_address_type,
+                                   lldb::addr_t caller_address,
+                                   bool is_tail_call,
+                                   CallSiteParameterArray &&parameters)
+    : CallEdge(caller_address_type, caller_address, is_tail_call,
+               std::move(parameters)),
+      call_target(std::move(call_target)) {}
+
 Function *IndirectCallEdge::GetCallee(ModuleList &images,
                                       ExecutionContext &exe_ctx) {
   Log *log = GetLog(LLDBLog::Step);

Copy link
Member

@bulbazord bulbazord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, if we're defining a lot of the methods for these classes in implementation files, we should do the same for constructors and destructors too.

This prevents undefined vtable errors when linking these libraries from out-of-tree. I'm facing this issue as I work on my new language plugin.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants