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] BreakpointResolver{*}::CreateFromStructuredData should return shared pointers #71477

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

bulbazord
Copy link
Member

BreakpointResolver::CreateFromStructuredData returns a BreakpointResolverSP, but all of the subclasses return raw pointers. Instead of creating a raw pointer and shoving it into a shared pointer, it seems reasonable to just create the shared pointer directly.

…shared pointers

BreakpointResolver::CreateFromStructuredData returns a
BreakpointResolverSP, but all of the subclasses return raw pointers.
Instead of creating a raw pointer and shoving it into a shared pointer,
it seems reasonable to just create the shared pointer directly.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 7, 2023

@llvm/pr-subscribers-lldb

Author: Alex Langford (bulbazord)

Changes

BreakpointResolver::CreateFromStructuredData returns a BreakpointResolverSP, but all of the subclasses return raw pointers. Instead of creating a raw pointer and shoving it into a shared pointer, it seems reasonable to just create the shared pointer directly.


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

11 Files Affected:

  • (modified) lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h (+1-1)
  • (modified) lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h (+1-1)
  • (modified) lldb/include/lldb/Breakpoint/BreakpointResolverFileRegex.h (+1-1)
  • (modified) lldb/include/lldb/Breakpoint/BreakpointResolverName.h (+1-1)
  • (modified) lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h (+1-1)
  • (modified) lldb/source/Breakpoint/BreakpointResolver.cpp (+9-10)
  • (modified) lldb/source/Breakpoint/BreakpointResolverAddress.cpp (+3-2)
  • (modified) lldb/source/Breakpoint/BreakpointResolverFileLine.cpp (+3-3)
  • (modified) lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp (+5-5)
  • (modified) lldb/source/Breakpoint/BreakpointResolverName.cpp (+9-8)
  • (modified) lldb/source/Breakpoint/BreakpointResolverScripted.cpp (+4-5)
diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h b/lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h
index 5454487e51a234c..03ae69acae4c4fd 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h
@@ -30,7 +30,7 @@ class BreakpointResolverAddress : public BreakpointResolver {
 
   ~BreakpointResolverAddress() override = default;
 
-  static BreakpointResolver *
+  static lldb::BreakpointResolverSP
   CreateFromStructuredData(const lldb::BreakpointSP &bkpt,
                            const StructuredData::Dictionary &options_dict,
                            Status &error);
diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h b/lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
index 3747e6d2d9a22ad..7635729c50a6e06 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
@@ -27,7 +27,7 @@ class BreakpointResolverFileLine : public BreakpointResolver {
       const SourceLocationSpec &location_spec,
       std::optional<llvm::StringRef> removed_prefix_opt = std::nullopt);
 
-  static BreakpointResolver *
+  static lldb::BreakpointResolverSP
   CreateFromStructuredData(const lldb::BreakpointSP &bkpt,
                            const StructuredData::Dictionary &data_dict,
                            Status &error);
diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolverFileRegex.h b/lldb/include/lldb/Breakpoint/BreakpointResolverFileRegex.h
index 138d555e2230743..43e1217c13d5ef6 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointResolverFileRegex.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointResolverFileRegex.h
@@ -27,7 +27,7 @@ class BreakpointResolverFileRegex : public BreakpointResolver {
       const lldb::BreakpointSP &bkpt, RegularExpression regex,
       const std::unordered_set<std::string> &func_name_set, bool exact_match);
 
-  static BreakpointResolver *
+  static lldb::BreakpointResolverSP
   CreateFromStructuredData(const lldb::BreakpointSP &bkpt,
                            const StructuredData::Dictionary &options_dict,
                            Status &error);
diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolverName.h b/lldb/include/lldb/Breakpoint/BreakpointResolverName.h
index cbfcbc9af0ea1cb..94b19db3085d768 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointResolverName.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointResolverName.h
@@ -50,7 +50,7 @@ class BreakpointResolverName : public BreakpointResolver {
                          lldb::LanguageType language, lldb::addr_t offset,
                          bool skip_prologue);
 
-  static BreakpointResolver *
+  static lldb::BreakpointResolverSP
   CreateFromStructuredData(const lldb::BreakpointSP &bkpt,
                            const StructuredData::Dictionary &data_dict,
                            Status &error);
diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h b/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h
index cecd0f92a21a86c..c0bbc1c2bafb704 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h
@@ -30,7 +30,7 @@ class BreakpointResolverScripted : public BreakpointResolver {
 
   ~BreakpointResolverScripted() override = default;
 
-  static BreakpointResolver *
+  static lldb::BreakpointResolverSP
   CreateFromStructuredData(const lldb::BreakpointSP &bkpt,
                            const StructuredData::Dictionary &options_dict,
                            Status &error);
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp
index c1e77d3c70da103..60406bdc44625d3 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -110,26 +110,25 @@ BreakpointResolverSP BreakpointResolver::CreateFromStructuredData(
     return result_sp;
   }
 
-  BreakpointResolver *resolver = nullptr;
   switch (resolver_type) {
   case FileLineResolver:
-    resolver = BreakpointResolverFileLine::CreateFromStructuredData(
+    result_sp = BreakpointResolverFileLine::CreateFromStructuredData(
         nullptr, *subclass_options, error);
     break;
   case AddressResolver:
-    resolver = BreakpointResolverAddress::CreateFromStructuredData(
+    result_sp = BreakpointResolverAddress::CreateFromStructuredData(
         nullptr, *subclass_options, error);
     break;
   case NameResolver:
-    resolver = BreakpointResolverName::CreateFromStructuredData(
+    result_sp = BreakpointResolverName::CreateFromStructuredData(
         nullptr, *subclass_options, error);
     break;
   case FileRegexResolver:
-    resolver = BreakpointResolverFileRegex::CreateFromStructuredData(
+    result_sp = BreakpointResolverFileRegex::CreateFromStructuredData(
         nullptr, *subclass_options, error);
     break;
   case PythonResolver:
-    resolver = BreakpointResolverScripted::CreateFromStructuredData(
+    result_sp = BreakpointResolverScripted::CreateFromStructuredData(
         nullptr, *subclass_options, error);
     break;
   case ExceptionResolver:
@@ -139,12 +138,12 @@ BreakpointResolverSP BreakpointResolver::CreateFromStructuredData(
     llvm_unreachable("Should never get an unresolvable resolver type.");
   }
 
-  if (!resolver || error.Fail())
-    return result_sp;
+  if (error.Fail() || !result_sp)
+    return {};
 
   // Add on the global offset option:
-  resolver->SetOffset(offset);
-  return BreakpointResolverSP(resolver);
+  result_sp->SetOffset(offset);
+  return result_sp;
 }
 
 StructuredData::DictionarySP BreakpointResolver::WrapOptionsDict(
diff --git a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp
index 26f289103bb88ba..19fc81a28a7f4af 100644
--- a/lldb/source/Breakpoint/BreakpointResolverAddress.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverAddress.cpp
@@ -30,7 +30,7 @@ BreakpointResolverAddress::BreakpointResolverAddress(const BreakpointSP &bkpt,
     : BreakpointResolver(bkpt, BreakpointResolver::AddressResolver),
       m_addr(addr), m_resolved_addr(LLDB_INVALID_ADDRESS) {}
 
-BreakpointResolver *BreakpointResolverAddress::CreateFromStructuredData(
+BreakpointResolverSP BreakpointResolverAddress::CreateFromStructuredData(
     const BreakpointSP &bkpt, const StructuredData::Dictionary &options_dict,
     Status &error) {
   llvm::StringRef module_name;
@@ -56,7 +56,8 @@ BreakpointResolver *BreakpointResolverAddress::CreateFromStructuredData(
     }
     module_filespec.SetFile(module_name, FileSpec::Style::native);
   }
-  return new BreakpointResolverAddress(bkpt, address, module_filespec);
+  return std::make_shared<BreakpointResolverAddress>(bkpt, address,
+                                                     module_filespec);
 }
 
 StructuredData::ObjectSP
diff --git a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
index 04374decd36350e..c28b78a0056f9ab 100644
--- a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
@@ -30,7 +30,7 @@ BreakpointResolverFileLine::BreakpointResolverFileLine(
       m_location_spec(location_spec), m_skip_prologue(skip_prologue),
       m_removed_prefix_opt(removed_prefix_opt) {}
 
-BreakpointResolver *BreakpointResolverFileLine::CreateFromStructuredData(
+BreakpointResolverSP BreakpointResolverFileLine::CreateFromStructuredData(
     const BreakpointSP &bkpt, const StructuredData::Dictionary &options_dict,
     Status &error) {
   llvm::StringRef filename;
@@ -90,8 +90,8 @@ BreakpointResolver *BreakpointResolverFileLine::CreateFromStructuredData(
   if (!location_spec)
     return nullptr;
 
-  return new BreakpointResolverFileLine(bkpt, offset, skip_prologue,
-                                        location_spec);
+  return std::make_shared<BreakpointResolverFileLine>(
+      bkpt, offset, skip_prologue, location_spec);
 }
 
 StructuredData::ObjectSP
diff --git a/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp b/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
index cb3ee5fb74277bc..38de16a56155e96 100644
--- a/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
@@ -26,9 +26,9 @@ BreakpointResolverFileRegex::BreakpointResolverFileRegex(
       m_regex(std::move(regex)), m_exact_match(exact_match),
       m_function_names(func_names) {}
 
-BreakpointResolver *BreakpointResolverFileRegex::CreateFromStructuredData(
-    const lldb::BreakpointSP &bkpt, const StructuredData::Dictionary &options_dict,
-    Status &error) {
+BreakpointResolverSP BreakpointResolverFileRegex::CreateFromStructuredData(
+    const lldb::BreakpointSP &bkpt,
+    const StructuredData::Dictionary &options_dict, Status &error) {
   bool success;
 
   llvm::StringRef regex_string;
@@ -67,8 +67,8 @@ BreakpointResolver *BreakpointResolverFileRegex::CreateFromStructuredData(
     }
   }
 
-  return new BreakpointResolverFileRegex(bkpt, std::move(regex), names_set,
-                                         exact_match);
+  return std::make_shared<BreakpointResolverFileRegex>(bkpt, std::move(regex),
+                                                       names_set, exact_match);
 }
 
 StructuredData::ObjectSP
diff --git a/lldb/source/Breakpoint/BreakpointResolverName.cpp b/lldb/source/Breakpoint/BreakpointResolverName.cpp
index 861377a5ddabfb5..ef61df51ba16600 100644
--- a/lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -86,7 +86,7 @@ BreakpointResolverName::BreakpointResolverName(
       m_regex(rhs.m_regex), m_match_type(rhs.m_match_type),
       m_language(rhs.m_language), m_skip_prologue(rhs.m_skip_prologue) {}
 
-BreakpointResolver *BreakpointResolverName::CreateFromStructuredData(
+BreakpointResolverSP BreakpointResolverName::CreateFromStructuredData(
     const BreakpointSP &bkpt, const StructuredData::Dictionary &options_dict,
     Status &error) {
   LanguageType language = eLanguageTypeUnknown;
@@ -122,8 +122,8 @@ BreakpointResolver *BreakpointResolverName::CreateFromStructuredData(
   success = options_dict.GetValueForKeyAsString(
       GetKey(OptionNames::RegexString), regex_text);
   if (success) {
-    return new BreakpointResolverName(bkpt, RegularExpression(regex_text),
-                                      language, offset, skip_prologue);
+    return std::make_shared<BreakpointResolverName>(
+        bkpt, RegularExpression(regex_text), language, offset, skip_prologue);
   } else {
     StructuredData::Array *names_array;
     success = options_dict.GetValueForKeyAsArray(
@@ -172,13 +172,14 @@ BreakpointResolver *BreakpointResolverName::CreateFromStructuredData(
       name_masks.push_back(static_cast<FunctionNameType>(fnt));
     }
 
-    BreakpointResolverName *resolver = new BreakpointResolverName(
-        bkpt, names[0].c_str(), name_masks[0], language,
-        Breakpoint::MatchType::Exact, offset, skip_prologue);
+    std::shared_ptr<BreakpointResolverName> resolver_sp =
+        std::make_shared<BreakpointResolverName>(
+            bkpt, names[0].c_str(), name_masks[0], language,
+            Breakpoint::MatchType::Exact, offset, skip_prologue);
     for (size_t i = 1; i < num_elem; i++) {
-      resolver->AddNameLookup(ConstString(names[i]), name_masks[i]);
+      resolver_sp->AddNameLookup(ConstString(names[i]), name_masks[i]);
     }
-    return resolver;
+    return resolver_sp;
   }
 }
 
diff --git a/lldb/source/Breakpoint/BreakpointResolverScripted.cpp b/lldb/source/Breakpoint/BreakpointResolverScripted.cpp
index 308c3b987f581d3..664ce4d573f943f 100644
--- a/lldb/source/Breakpoint/BreakpointResolverScripted.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverScripted.cpp
@@ -58,13 +58,12 @@ void BreakpointResolverScripted::NotifyBreakpointSet() {
   CreateImplementationIfNeeded(GetBreakpoint());
 }
 
-BreakpointResolver *
-BreakpointResolverScripted::CreateFromStructuredData(
+BreakpointResolverSP BreakpointResolverScripted::CreateFromStructuredData(
     const BreakpointSP &bkpt, const StructuredData::Dictionary &options_dict,
     Status &error) {
   llvm::StringRef class_name;
   bool success;
-  
+
   success = options_dict.GetValueForKeyAsString(
       GetKey(OptionNames::PythonClassName), class_name);
   if (!success) {
@@ -80,8 +79,8 @@ BreakpointResolverScripted::CreateFromStructuredData(
   if (options_dict.GetValueForKeyAsDictionary(GetKey(OptionNames::ScriptArgs),
                                               args_dict))
     args_data_impl.SetObjectSP(args_dict->shared_from_this());
-  return new BreakpointResolverScripted(bkpt, class_name, depth, 
-                                        args_data_impl);
+  return std::make_shared<BreakpointResolverScripted>(bkpt, class_name, depth,
+                                                      args_data_impl);
 }
 
 StructuredData::ObjectSP

Copy link
Member

@medismailben medismailben left a comment

Choose a reason for hiding this comment

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

LGTM!

@bulbazord bulbazord merged commit 03a92f0 into llvm:main Nov 7, 2023
4 checks passed
@bulbazord bulbazord deleted the no-more-raw-pointers branch November 7, 2023 19:22
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