Skip to content

Conversation

@JDevlieghere
Copy link
Member

Use the if statement with an initializer pattern that's very common in LLVM in SBTarget. Every time someone adds a new method to SBTarget, I want to encourage using this pattern, but I don't because it would be inconsistent with the rest of the file. This solves that problem by switching over the whole file.

@llvmbot
Copy link
Member

llvmbot commented May 23, 2025

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Use the if statement with an initializer pattern that's very common in LLVM in SBTarget. Every time someone adds a new method to SBTarget, I want to encourage using this pattern, but I don't because it would be inconsistent with the rest of the file. This solves that problem by switching over the whole file.


Patch is 60.76 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/141284.diff

1 Files Affected:

  • (modified) lldb/source/API/SBTarget.cpp (+436-508)
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index cd8a770a0ec04..6363677bedf6e 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -166,8 +166,7 @@ SBProcess SBTarget::GetProcess() {
 
   SBProcess sb_process;
   ProcessSP process_sp;
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     process_sp = target_sp->GetProcessSP();
     sb_process.SetSP(process_sp);
   }
@@ -178,22 +177,19 @@ SBProcess SBTarget::GetProcess() {
 SBPlatform SBTarget::GetPlatform() {
   LLDB_INSTRUMENT_VA(this);
 
-  TargetSP target_sp(GetSP());
-  if (!target_sp)
-    return SBPlatform();
-
-  SBPlatform platform;
-  platform.m_opaque_sp = target_sp->GetPlatform();
-
-  return platform;
+  if (TargetSP target_sp = GetSP()) {
+    SBPlatform platform;
+    platform.m_opaque_sp = target_sp->GetPlatform();
+    return platform;
+  }
+  return SBPlatform();
 }
 
 SBDebugger SBTarget::GetDebugger() const {
   LLDB_INSTRUMENT_VA(this);
 
   SBDebugger debugger;
-  TargetSP target_sp(GetSP());
-  if (target_sp)
+  if (TargetSP target_sp = GetSP())
     debugger.reset(target_sp->GetDebugger().shared_from_this());
   return debugger;
 }
@@ -208,41 +204,37 @@ SBStructuredData SBTarget::GetStatistics(SBStatisticsOptions options) {
   LLDB_INSTRUMENT_VA(this);
 
   SBStructuredData data;
-  TargetSP target_sp(GetSP());
-  if (!target_sp)
+  if (TargetSP target_sp = GetSP()) {
+    std::string json_str =
+        llvm::formatv("{0:2}", DebuggerStats::ReportStatistics(
+                                   target_sp->GetDebugger(), target_sp.get(),
+                                   options.ref()))
+            .str();
+    data.m_impl_up->SetObjectSP(StructuredData::ParseJSON(json_str));
     return data;
-  std::string json_str =
-      llvm::formatv("{0:2}", DebuggerStats::ReportStatistics(
-                                 target_sp->GetDebugger(), target_sp.get(),
-                                 options.ref()))
-          .str();
-  data.m_impl_up->SetObjectSP(StructuredData::ParseJSON(json_str));
+  }
   return data;
 }
 
 void SBTarget::ResetStatistics() {
   LLDB_INSTRUMENT_VA(this);
-  TargetSP target_sp(GetSP());
-  if (target_sp)
+  if (TargetSP target_sp = GetSP())
     DebuggerStats::ResetStatistics(target_sp->GetDebugger(), target_sp.get());
 }
 
 void SBTarget::SetCollectingStats(bool v) {
   LLDB_INSTRUMENT_VA(this, v);
 
-  TargetSP target_sp(GetSP());
-  if (!target_sp)
-    return;
-  return DebuggerStats::SetCollectingStats(v);
+  if (TargetSP target_sp = GetSP())
+    return DebuggerStats::SetCollectingStats(v);
 }
 
 bool SBTarget::GetCollectingStats() {
   LLDB_INSTRUMENT_VA(this);
 
-  TargetSP target_sp(GetSP());
-  if (!target_sp)
-    return false;
-  return DebuggerStats::GetCollectingStats();
+  if (TargetSP target_sp = GetSP())
+    return DebuggerStats::GetCollectingStats();
+  return false;
 }
 
 SBProcess SBTarget::LoadCore(const char *core_file) {
@@ -256,8 +248,7 @@ SBProcess SBTarget::LoadCore(const char *core_file, lldb::SBError &error) {
   LLDB_INSTRUMENT_VA(this, core_file, error);
 
   SBProcess sb_process;
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     FileSpec filespec(core_file);
     FileSystem::Instance().Resolve(filespec);
     ProcessSP process_sp(target_sp->CreateProcess(
@@ -303,8 +294,7 @@ SBError SBTarget::Install() {
   LLDB_INSTRUMENT_VA(this);
 
   SBError sb_error;
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     sb_error.ref() = target_sp->Install(nullptr);
   }
@@ -323,9 +313,7 @@ SBProcess SBTarget::Launch(SBListener &listener, char const **argv,
 
   SBProcess sb_process;
   ProcessSP process_sp;
-  TargetSP target_sp(GetSP());
-
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
 
     if (stop_at_entry)
@@ -400,9 +388,7 @@ SBProcess SBTarget::Launch(SBLaunchInfo &sb_launch_info, SBError &error) {
   LLDB_INSTRUMENT_VA(this, sb_launch_info, error);
 
   SBProcess sb_process;
-  TargetSP target_sp(GetSP());
-
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     StateType state = eStateInvalid;
     {
@@ -446,9 +432,7 @@ lldb::SBProcess SBTarget::Attach(SBAttachInfo &sb_attach_info, SBError &error) {
   LLDB_INSTRUMENT_VA(this, sb_attach_info, error);
 
   SBProcess sb_process;
-  TargetSP target_sp(GetSP());
-
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     ProcessAttachInfo &attach_info = sb_attach_info.ref();
     if (attach_info.ProcessIDIsValid() && !attach_info.UserIDIsValid() &&
         !attach_info.IsScriptedProcess()) {
@@ -484,9 +468,7 @@ lldb::SBProcess SBTarget::AttachToProcessWithID(
   LLDB_INSTRUMENT_VA(this, listener, pid, error);
 
   SBProcess sb_process;
-  TargetSP target_sp(GetSP());
-
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     ProcessAttachInfo attach_info;
     attach_info.SetProcessID(pid);
     if (listener.IsValid())
@@ -514,9 +496,13 @@ lldb::SBProcess SBTarget::AttachToProcessWithName(
   LLDB_INSTRUMENT_VA(this, listener, name, wait_for, error);
 
   SBProcess sb_process;
-  TargetSP target_sp(GetSP());
 
-  if (name && target_sp) {
+  if (!name) {
+    error.SetErrorString("invalid name");
+    return sb_process;
+  }
+
+  if (TargetSP target_sp = GetSP()) {
     ProcessAttachInfo attach_info;
     attach_info.GetExecutableFile().SetFile(name, FileSpec::Style::native);
     attach_info.SetWaitForLaunch(wait_for);
@@ -526,8 +512,9 @@ lldb::SBProcess SBTarget::AttachToProcessWithName(
     error.SetError(AttachToProcess(attach_info, *target_sp));
     if (error.Success())
       sb_process.SetSP(target_sp->GetProcessSP());
-  } else
+  } else {
     error.SetErrorString("SBTarget is invalid");
+  }
 
   return sb_process;
 }
@@ -539,9 +526,7 @@ lldb::SBProcess SBTarget::ConnectRemote(SBListener &listener, const char *url,
 
   SBProcess sb_process;
   ProcessSP process_sp;
-  TargetSP target_sp(GetSP());
-
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     if (listener.IsValid())
       process_sp =
@@ -568,8 +553,7 @@ SBFileSpec SBTarget::GetExecutable() {
   LLDB_INSTRUMENT_VA(this);
 
   SBFileSpec exe_file_spec;
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     Module *exe_module = target_sp->GetExecutableModulePointer();
     if (exe_module)
       exe_file_spec.SetFileSpec(exe_module->GetFileSpec());
@@ -601,8 +585,7 @@ lldb::SBAddress SBTarget::ResolveLoadAddress(lldb::addr_t vm_addr) {
 
   lldb::SBAddress sb_addr;
   Address &addr = sb_addr.ref();
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     if (target_sp->ResolveLoadAddress(vm_addr, addr))
       return sb_addr;
@@ -619,8 +602,7 @@ lldb::SBAddress SBTarget::ResolveFileAddress(lldb::addr_t file_addr) {
 
   lldb::SBAddress sb_addr;
   Address &addr = sb_addr.ref();
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     if (target_sp->ResolveFileAddress(file_addr, addr))
       return sb_addr;
@@ -636,8 +618,7 @@ lldb::SBAddress SBTarget::ResolvePastLoadAddress(uint32_t stop_id,
 
   lldb::SBAddress sb_addr;
   Address &addr = sb_addr.ref();
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     if (target_sp->ResolveLoadAddress(vm_addr, addr))
       return sb_addr;
@@ -657,8 +638,7 @@ SBTarget::ResolveSymbolContextForAddress(const SBAddress &addr,
   SBSymbolContext sc;
   SymbolContextItem scope = static_cast<SymbolContextItem>(resolve_scope);
   if (addr.IsValid()) {
-    TargetSP target_sp(GetSP());
-    if (target_sp)
+    if (TargetSP target_sp = GetSP())
       target_sp->GetImages().ResolveSymbolContextForAddress(addr.ref(), scope,
                                                             sc.ref());
   }
@@ -670,8 +650,7 @@ size_t SBTarget::ReadMemory(const SBAddress addr, void *buf, size_t size,
   LLDB_INSTRUMENT_VA(this, addr, buf, size, error);
 
   size_t bytes_read = 0;
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     bytes_read =
         target_sp->ReadMemory(addr.ref(), buf, size, error.ref(), true);
@@ -723,22 +702,23 @@ SBBreakpoint SBTarget::BreakpointCreateByLocation(
   LLDB_INSTRUMENT_VA(this, sb_file_spec, line, column, offset, sb_module_list);
 
   SBBreakpoint sb_bp;
-  TargetSP target_sp(GetSP());
-  if (target_sp && line != 0) {
-    std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
-
-    const LazyBool check_inlines = eLazyBoolCalculate;
-    const LazyBool skip_prologue = eLazyBoolCalculate;
-    const bool internal = false;
-    const bool hardware = false;
-    const LazyBool move_to_nearest_code = eLazyBoolCalculate;
-    const FileSpecList *module_list = nullptr;
-    if (sb_module_list.GetSize() > 0) {
-      module_list = sb_module_list.get();
+  if (TargetSP target_sp = GetSP()) {
+    if (line != 0) {
+      std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
+
+      const LazyBool check_inlines = eLazyBoolCalculate;
+      const LazyBool skip_prologue = eLazyBoolCalculate;
+      const bool internal = false;
+      const bool hardware = false;
+      const LazyBool move_to_nearest_code = eLazyBoolCalculate;
+      const FileSpecList *module_list = nullptr;
+      if (sb_module_list.GetSize() > 0) {
+        module_list = sb_module_list.get();
+      }
+      sb_bp = target_sp->CreateBreakpoint(
+          module_list, *sb_file_spec, line, column, offset, check_inlines,
+          skip_prologue, internal, hardware, move_to_nearest_code);
     }
-    sb_bp = target_sp->CreateBreakpoint(
-        module_list, *sb_file_spec, line, column, offset, check_inlines,
-        skip_prologue, internal, hardware, move_to_nearest_code);
   }
 
   return sb_bp;
@@ -752,22 +732,23 @@ SBBreakpoint SBTarget::BreakpointCreateByLocation(
                      move_to_nearest_code);
 
   SBBreakpoint sb_bp;
-  TargetSP target_sp(GetSP());
-  if (target_sp && line != 0) {
-    std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
-
-    const LazyBool check_inlines = eLazyBoolCalculate;
-    const LazyBool skip_prologue = eLazyBoolCalculate;
-    const bool internal = false;
-    const bool hardware = false;
-    const FileSpecList *module_list = nullptr;
-    if (sb_module_list.GetSize() > 0) {
-      module_list = sb_module_list.get();
+  if (TargetSP target_sp = GetSP()) {
+    if (line != 0) {
+      std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
+
+      const LazyBool check_inlines = eLazyBoolCalculate;
+      const LazyBool skip_prologue = eLazyBoolCalculate;
+      const bool internal = false;
+      const bool hardware = false;
+      const FileSpecList *module_list = nullptr;
+      if (sb_module_list.GetSize() > 0) {
+        module_list = sb_module_list.get();
+      }
+      sb_bp = target_sp->CreateBreakpoint(
+          module_list, *sb_file_spec, line, column, offset, check_inlines,
+          skip_prologue, internal, hardware,
+          move_to_nearest_code ? eLazyBoolYes : eLazyBoolNo);
     }
-    sb_bp = target_sp->CreateBreakpoint(
-        module_list, *sb_file_spec, line, column, offset, check_inlines,
-        skip_prologue, internal, hardware,
-        move_to_nearest_code ? eLazyBoolYes : eLazyBoolNo);
   }
 
   return sb_bp;
@@ -778,8 +759,7 @@ SBBreakpoint SBTarget::BreakpointCreateByName(const char *symbol_name,
   LLDB_INSTRUMENT_VA(this, symbol_name, module_name);
 
   SBBreakpoint sb_bp;
-  TargetSP target_sp(GetSP());
-  if (target_sp.get()) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
 
     const bool internal = false;
@@ -833,16 +813,17 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateByName(
                      module_list, comp_unit_list);
 
   SBBreakpoint sb_bp;
-  TargetSP target_sp(GetSP());
-  if (target_sp && symbol_name && symbol_name[0]) {
-    const bool internal = false;
-    const bool hardware = false;
-    const LazyBool skip_prologue = eLazyBoolCalculate;
-    std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
-    FunctionNameType mask = static_cast<FunctionNameType>(name_type_mask);
-    sb_bp = target_sp->CreateBreakpoint(module_list.get(), comp_unit_list.get(),
-                                        symbol_name, mask, symbol_language, 0,
-                                        skip_prologue, internal, hardware);
+  if (TargetSP target_sp = GetSP()) {
+    if (symbol_name && symbol_name[0]) {
+      const bool internal = false;
+      const bool hardware = false;
+      const LazyBool skip_prologue = eLazyBoolCalculate;
+      std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
+      FunctionNameType mask = static_cast<FunctionNameType>(name_type_mask);
+      sb_bp = target_sp->CreateBreakpoint(
+          module_list.get(), comp_unit_list.get(), symbol_name, mask,
+          symbol_language, 0, skip_prologue, internal, hardware);
+    }
   }
 
   return sb_bp;
@@ -879,16 +860,17 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateByNames(
                      symbol_language, offset, module_list, comp_unit_list);
 
   SBBreakpoint sb_bp;
-  TargetSP target_sp(GetSP());
-  if (target_sp && num_names > 0) {
-    std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
-    const bool internal = false;
-    const bool hardware = false;
-    FunctionNameType mask = static_cast<FunctionNameType>(name_type_mask);
-    const LazyBool skip_prologue = eLazyBoolCalculate;
-    sb_bp = target_sp->CreateBreakpoint(
-        module_list.get(), comp_unit_list.get(), symbol_names, num_names, mask,
-        symbol_language, offset, skip_prologue, internal, hardware);
+  if (TargetSP target_sp = GetSP()) {
+    if (num_names > 0) {
+      std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
+      const bool internal = false;
+      const bool hardware = false;
+      FunctionNameType mask = static_cast<FunctionNameType>(name_type_mask);
+      const LazyBool skip_prologue = eLazyBoolCalculate;
+      sb_bp = target_sp->CreateBreakpoint(
+          module_list.get(), comp_unit_list.get(), symbol_names, num_names,
+          mask, symbol_language, offset, skip_prologue, internal, hardware);
+    }
   }
 
   return sb_bp;
@@ -924,17 +906,18 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateByRegex(
                      comp_unit_list);
 
   SBBreakpoint sb_bp;
-  TargetSP target_sp(GetSP());
-  if (target_sp && symbol_name_regex && symbol_name_regex[0]) {
-    std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
-    RegularExpression regexp((llvm::StringRef(symbol_name_regex)));
-    const bool internal = false;
-    const bool hardware = false;
-    const LazyBool skip_prologue = eLazyBoolCalculate;
-
-    sb_bp = target_sp->CreateFuncRegexBreakpoint(
-        module_list.get(), comp_unit_list.get(), std::move(regexp),
-        symbol_language, skip_prologue, internal, hardware);
+  if (TargetSP target_sp = GetSP()) {
+    if (symbol_name_regex && symbol_name_regex[0]) {
+      std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
+      RegularExpression regexp((llvm::StringRef(symbol_name_regex)));
+      const bool internal = false;
+      const bool hardware = false;
+      const LazyBool skip_prologue = eLazyBoolCalculate;
+
+      sb_bp = target_sp->CreateFuncRegexBreakpoint(
+          module_list.get(), comp_unit_list.get(), std::move(regexp),
+          symbol_language, skip_prologue, internal, hardware);
+    }
   }
 
   return sb_bp;
@@ -944,8 +927,7 @@ SBBreakpoint SBTarget::BreakpointCreateByAddress(addr_t address) {
   LLDB_INSTRUMENT_VA(this, address);
 
   SBBreakpoint sb_bp;
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     const bool hardware = false;
     sb_bp = target_sp->CreateBreakpoint(address, false, hardware);
@@ -958,12 +940,11 @@ SBBreakpoint SBTarget::BreakpointCreateBySBAddress(SBAddress &sb_address) {
   LLDB_INSTRUMENT_VA(this, sb_address);
 
   SBBreakpoint sb_bp;
-  TargetSP target_sp(GetSP());
   if (!sb_address.IsValid()) {
     return sb_bp;
   }
 
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     const bool hardware = false;
     sb_bp = target_sp->CreateBreakpoint(sb_address.ref(), false, hardware);
@@ -1010,20 +991,21 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateBySourceRegex(
                      func_names);
 
   SBBreakpoint sb_bp;
-  TargetSP target_sp(GetSP());
-  if (target_sp && source_regex && source_regex[0]) {
-    std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
-    const bool hardware = false;
-    const LazyBool move_to_nearest_code = eLazyBoolCalculate;
-    RegularExpression regexp((llvm::StringRef(source_regex)));
-    std::unordered_set<std::string> func_names_set;
-    for (size_t i = 0; i < func_names.GetSize(); i++) {
-      func_names_set.insert(func_names.GetStringAtIndex(i));
-    }
+  if (TargetSP target_sp = GetSP()) {
+    if (source_regex && source_regex[0]) {
+      std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
+      const bool hardware = false;
+      const LazyBool move_to_nearest_code = eLazyBoolCalculate;
+      RegularExpression regexp((llvm::StringRef(source_regex)));
+      std::unordered_set<std::string> func_names_set;
+      for (size_t i = 0; i < func_names.GetSize(); i++) {
+        func_names_set.insert(func_names.GetStringAtIndex(i));
+      }
 
-    sb_bp = target_sp->CreateSourceRegexBreakpoint(
-        module_list.get(), source_file_list.get(), func_names_set,
-        std::move(regexp), false, hardware, move_to_nearest_code);
+      sb_bp = target_sp->CreateSourceRegexBreakpoint(
+          module_list.get(), source_file_list.get(), func_names_set,
+          std::move(regexp), false, hardware, move_to_nearest_code);
+    }
   }
 
   return sb_bp;
@@ -1035,8 +1017,7 @@ SBTarget::BreakpointCreateForException(lldb::LanguageType language,
   LLDB_INSTRUMENT_VA(this, language, catch_bp, throw_bp);
 
   SBBreakpoint sb_bp;
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     const bool hardware = false;
     sb_bp = target_sp->CreateExceptionBreakpoint(language, catch_bp, throw_bp,
@@ -1054,8 +1035,7 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateFromScript(
                      request_hardware);
 
   SBBreakpoint sb_bp;
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     Status error;
 
@@ -1076,8 +1056,7 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateFromScript(
 uint32_t SBTarget::GetNumBreakpoints() const {
   LLDB_INSTRUMENT_VA(this);
 
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     // The breakpoint list is thread safe, no need to lock
     return target_sp->GetBreakpointList().GetSize();
   }
@@ -1088,8 +1067,7 @@ SBBreakpoint SBTarget::GetBreakpointAtIndex(uint32_t idx) const {
   LLDB_INSTRUMENT_VA(this, idx);
 
   SBBreakpoint sb_breakpoint;
-  TargetSP target_sp(GetSP());
-  if (target_sp) {
+  if (TargetSP target_sp = GetSP()) {
     // The breakpoint list is thread safe, no need to lock
     sb_breakpoint = target_sp->GetBreakpointList().GetBreakpointAtIndex(idx);
   }
@@ -1100,8 +1078,7 @@ bool SBTarget::BreakpointDelete(break_id_t bp_id) {
...
[truncated]

@JDevlieghere
Copy link
Member Author

The diff looks a whole lot less intimidating when ignoring whitespace changes.

@JDevlieghere JDevlieghere force-pushed the if-with-initializer branch 2 times, most recently from 5b85557 to bcc6dd2 Compare May 23, 2025 22:28
Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

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

I'm not sure this change is beneficial in the cases where it causes an increase in the level of indentation (due to separating the check for TargetSP from other validity checks).

What would you say to using the c++17 init-statement, plus condition?

Use the if statement with an initializer pattern that's very common in
LLVM in SBTarget. Every time someone adds a new method to SBTarget, I
want to encourage using this pattern, but I don't because it would be
inconsistent with the rest of the file. This solves that problem by
switching over the whole file.
@JDevlieghere JDevlieghere force-pushed the if-with-initializer branch from bcc6dd2 to 2735f5a Compare May 27, 2025 17:53
Comment on lines 1548 to 1549
if (TargetSP target_sp = GetSP()) {
if (sb_file_spec.IsValid()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

missed two :)

const TargetSP target_sp(GetSP());
if (target_sp && sb_file_spec.IsValid())
target_sp->GetImages().FindCompileUnits(*sb_file_spec, *sb_sc_list);
if (TargetSP target_sp = GetSP()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

and here

auto types = vendor->FindTypes(const_typename, /*max_matches*/ 1);
if (!types.empty())
return SBType(types.front());
if (TargetSP target_sp = GetSP()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

here

vendor->FindTypes(const_typename, /*max_matches*/ UINT32_MAX);
for (auto type : types)
sb_type_list.Append(SBType(type));
if (TargetSP target_sp = GetSP()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

h

ValueObjectVariable::Create(exe_scope, var_sp));
if (valobj_sp)
sb_value_list.Append(SBValue(valobj_sp));
if (TargetSP target_sp = GetSP()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

h

if (name && target_sp) {
llvm::StringRef name_ref(name);
VariableList variable_list;
if (TargetSP target_sp = GetSP()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

h

Comment on lines +2324 to +2326
if (TargetSP target_sp = GetSP()) {
ModuleSP module_sp(module.GetSP());
if (module_sp)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an interesting one, because this pattern comes into conflict with the "early exit" rule. For a small function like this, this is fine, but if it was longer, I'd say that the early exit should win.

@JDevlieghere JDevlieghere merged commit 8199f18 into llvm:main May 28, 2025
10 checks passed
@JDevlieghere JDevlieghere deleted the if-with-initializer branch May 28, 2025 22:11
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.

4 participants