[PATCH] gdb: modernize get_frame_pc_if_available

[PATCH] gdb: modernize get_frame_pc_if_available

[Guinevere Larsen]

The convenience function get_frame_pc_if_available would take a pointer
to a variable that should be set if available, and would return a
boolean indicating whether that action was successful or not.

Now that GDB supports C++17 features, this indirection of a pointer and
returning boolean is unnecessary, since the function can return an
optional, and code that calls it can check if the optional contains a
value.

This commit makes that modernization. It should have no visible
effects.
---
 gdb/frame.c                      | 32 +++++++++++++--------------
 gdb/frame.h                      |  3 ++-
 gdb/macroscope.c                 |  6 ++---
 gdb/printcmd.c                   |  4 ++--
 gdb/python/py-finishbreakpoint.c |  6 ++---
 gdb/stack.c                      | 38 +++++++++++++++-----------------
 gdb/tracepoint.c                 |  8 +++----
 gdb/tui/tui-status.c             |  8 +++++--
 gdb/tui/tui-winsource.c          |  4 +++-
 9 files changed, 56 insertions(+), 53 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index fc4cbca0bd2..f92e0962416 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2663,15 +2663,14 @@ get_prev_frame (const frame_info_ptr &this_frame)
 {
   FRAME_SCOPED_DEBUG_ENTER_EXIT;
 
-  CORE_ADDR frame_pc;
-  int frame_pc_p;
+  std::optional<CORE_ADDR> frame_pc;
 
   /* There is always a frame.  If this assertion fails, suspect that
      something should be calling get_selected_frame() or
      get_current_frame().  */
   gdb_assert (this_frame != NULL);
 
-  frame_pc_p = get_frame_pc_if_available (this_frame, &frame_pc);
+  frame_pc = get_frame_pc_if_available (this_frame);
 
   /* tausq/2004-12-07: Dummy frames are skipped because it doesn't make much
      sense to stop unwinding at a dummy frame.  One place where a dummy
@@ -2686,7 +2685,7 @@ get_prev_frame (const frame_info_ptr &this_frame)
   if (this_frame->level >= 0
       && get_frame_type (this_frame) == NORMAL_FRAME
       && !user_set_backtrace_options.backtrace_past_main
-      && frame_pc_p
+      && frame_pc
       && inside_main_func (this_frame))
     /* Don't unwind past main().  Note, this is done _before_ the
        frame has been marked as previously unwound.  That way if the
@@ -2733,7 +2732,7 @@ get_prev_frame (const frame_info_ptr &this_frame)
   if (this_frame->level >= 0
       && get_frame_type (this_frame) == NORMAL_FRAME
       && !user_set_backtrace_options.backtrace_past_entry
-      && frame_pc_p
+      && frame_pc
       && inside_entry_func (this_frame))
     {
       frame_debug_got_null_frame (this_frame, "inside entry func");
@@ -2747,7 +2746,7 @@ get_prev_frame (const frame_info_ptr &this_frame)
       && (get_frame_type (this_frame) == NORMAL_FRAME
 	  || get_frame_type (this_frame) == INLINE_FRAME)
       && get_frame_type (get_next_frame (this_frame)) == NORMAL_FRAME
-      && frame_pc_p && frame_pc == 0)
+      && frame_pc && *frame_pc == 0)
     {
       frame_debug_got_null_frame (this_frame, "zero PC");
       return NULL;
@@ -2763,25 +2762,24 @@ get_frame_pc (const frame_info_ptr &frame)
   return frame_unwind_pc (frame_info_ptr (frame->next));
 }
 
-bool
-get_frame_pc_if_available (const frame_info_ptr &frame, CORE_ADDR *pc)
+std::optional<CORE_ADDR>
+get_frame_pc_if_available (const frame_info_ptr &frame)
 {
+  std::optional<CORE_ADDR> pc;
 
   gdb_assert (frame->next != NULL);
 
   try
     {
-      *pc = frame_unwind_pc (frame_info_ptr (frame->next));
+      pc = frame_unwind_pc (frame_info_ptr (frame->next));
     }
   catch (const gdb_exception_error &ex)
     {
-      if (ex.error == NOT_AVAILABLE_ERROR)
-	return false;
-      else
+      if (ex.error != NOT_AVAILABLE_ERROR)
 	throw;
     }
 
-  return true;
+  return pc;
 }
 
 /* Return an address that falls within THIS_FRAME's code block.  */
@@ -2870,7 +2868,7 @@ find_frame_sal (const frame_info_ptr &frame)
 {
   frame_info_ptr next_frame;
   int notcurrent;
-  CORE_ADDR pc;
+  std::optional<CORE_ADDR> pc;
 
   if (frame_inlined_callees (frame) > 0)
     {
@@ -2914,11 +2912,11 @@ find_frame_sal (const frame_info_ptr &frame)
      PC and such a PC indicates the current (rather than next)
      instruction/line, consequently, for such cases, want to get the
      line containing fi->pc.  */
-  if (!get_frame_pc_if_available (frame, &pc))
+  if (!(pc = get_frame_pc_if_available (frame)))
     return {};
 
-  notcurrent = (pc != get_frame_address_in_block (frame));
-  return find_pc_line (pc, notcurrent);
+  notcurrent = (*pc != get_frame_address_in_block (frame));
+  return find_pc_line (*pc, notcurrent);
 }
 
 /* Per "frame.h", return the ``address'' of the frame.  Code should
diff --git a/gdb/frame.h b/gdb/frame.h
index b240662bd73..99a79835efa 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -504,7 +504,8 @@ extern CORE_ADDR get_frame_pc (const frame_info_ptr &);
 /* Same as get_frame_pc, but return a boolean indication of whether
    the PC is actually available, instead of throwing an error.  */
 
-extern bool get_frame_pc_if_available (const frame_info_ptr &frame, CORE_ADDR *pc);
+extern std::optional<CORE_ADDR> get_frame_pc_if_available
+  (const frame_info_ptr &frame);
 
 /* An address (not necessarily aligned to an instruction boundary)
    that falls within THIS frame's code block.
diff --git a/gdb/macroscope.c b/gdb/macroscope.c
index 7aa0784866a..a8b78c9906c 100644
--- a/gdb/macroscope.c
+++ b/gdb/macroscope.c
@@ -97,12 +97,12 @@ default_macro_scope ()
 {
   struct symtab_and_line sal;
   frame_info_ptr frame;
-  CORE_ADDR pc;
+  std::optional<CORE_ADDR> pc;
 
   /* If there's a selected frame, use its PC.  */
   frame = deprecated_safe_get_selected_frame ();
-  if (frame && get_frame_pc_if_available (frame, &pc))
-    sal = find_pc_line (pc, 0);
+  if (frame && (pc = get_frame_pc_if_available (frame)))
+    sal = find_pc_line (*pc, 0);
 
   /* Fall back to the current listing position.  */
   else
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index d1cf72d371d..c6d7075e89b 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -753,10 +753,10 @@ pc_prefix (CORE_ADDR addr)
   if (has_stack_frames ())
     {
       frame_info_ptr frame;
-      CORE_ADDR pc;
+      std::optional<CORE_ADDR> pc;
 
       frame = get_selected_frame (NULL);
-      if (get_frame_pc_if_available (frame, &pc) && pc == addr)
+      if ((pc = get_frame_pc_if_available (frame)) && *pc == addr)
 	return "=> ";
     }
   return "   ";
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 0ea629f08bd..70e1684524d 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -175,7 +175,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
   struct frame_id frame_id;
   PyObject *internal = NULL;
   int internal_bp = 0;
-  CORE_ADDR pc;
+  std::optional<CORE_ADDR> pc;
 
   if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "|OO", keywords,
 					&frame_obj, &internal))
@@ -249,9 +249,9 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 
   try
     {
-      if (get_frame_pc_if_available (frame, &pc))
+      if ((pc = get_frame_pc_if_available (frame)))
 	{
-	  struct symbol *function = find_pc_function (pc);
+	  struct symbol *function = find_pc_function (*pc);
 	  if (function != nullptr)
 	    {
 	      struct type *ret_type =
diff --git a/gdb/stack.c b/gdb/stack.c
index e6335669531..703aedc5aa8 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1165,10 +1165,10 @@ do_print_frame_info (struct ui_out *uiout, const frame_print_options &fp_opts,
 
   if (set_current_sal)
     {
-      CORE_ADDR pc;
+      std::optional<CORE_ADDR> pc;
 
-      if (get_frame_pc_if_available (frame, &pc))
-	last_displayed_symtab_info.set (sal.pspace, pc, sal.symtab, sal.line);
+      if ((pc = get_frame_pc_if_available (frame)))
+	last_displayed_symtab_info.set (sal.pspace, *pc, sal.symtab, sal.line);
       else
 	last_displayed_symtab_info.invalidate ();
     }
@@ -1325,16 +1325,15 @@ print_frame (struct ui_out *uiout,
   enum language funlang = language_unknown;
   struct value_print_options opts;
   struct symbol *func;
-  CORE_ADDR pc = 0;
-  int pc_p;
+  std::optional <CORE_ADDR> pc;
 
-  pc_p = get_frame_pc_if_available (frame, &pc);
+  pc = get_frame_pc_if_available (frame);
 
   gdb::unique_xmalloc_ptr<char> funname
     = find_frame_funname (frame, &funlang, &func);
 
   annotate_frame_begin (print_level ? frame_relative_level (frame) : 0,
-			gdbarch, pc);
+			gdbarch, (pc) ? *pc : 0);
 
   {
     ui_out_emit_tuple tuple_emitter (uiout, "frame");
@@ -1352,8 +1351,8 @@ print_frame (struct ui_out *uiout,
 	  || print_what == LOC_AND_ADDRESS)
 	{
 	  annotate_frame_address ();
-	  if (pc_p)
-	    print_pc (uiout, gdbarch, frame, pc);
+	  if (pc)
+	    print_pc (uiout, gdbarch, frame, *pc);
 	  else
 	    uiout->field_string ("addr", "<unavailable>",
 				 metadata_style.style ());
@@ -1422,7 +1421,7 @@ print_frame (struct ui_out *uiout,
       }
 
     if (print_what != SHORT_LOCATION
-	&& pc_p && (funname == NULL || sal.symtab == NULL))
+	&& pc && (funname == NULL || sal.symtab == NULL))
       {
 	const char *lib
 	  = solib_name_from_address (get_frame_program_space (frame),
@@ -1481,8 +1480,7 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
   enum language funlang = language_unknown;
   const char *pc_regname;
   struct gdbarch *gdbarch;
-  CORE_ADDR frame_pc;
-  int frame_pc_p;
+  std::optional<CORE_ADDR> frame_pc;
   /* Initialize it to avoid "may be used uninitialized" warning.  */
   CORE_ADDR caller_pc = 0;
   int caller_pc_p = 0;
@@ -1503,7 +1501,7 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
        get_frame_pc().  */
     pc_regname = "pc";
 
-  frame_pc_p = get_frame_pc_if_available (fi, &frame_pc);
+  frame_pc = get_frame_pc_if_available (fi);
   func = get_frame_function (fi);
   symtab_and_line sal = find_frame_sal (fi);
   s = sal.symtab;
@@ -1525,9 +1523,9 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
 	    funname = func_only.get ();
 	}
     }
-  else if (frame_pc_p)
+  else if (frame_pc)
     {
-      bound_minimal_symbol msymbol = lookup_minimal_symbol_by_pc (frame_pc);
+      bound_minimal_symbol msymbol = lookup_minimal_symbol_by_pc (*frame_pc);
       if (msymbol.minsym != NULL)
 	{
 	  funname = msymbol.minsym->print_name ();
@@ -1548,7 +1546,7 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
   gdb_puts (paddress (gdbarch, get_frame_base (fi)));
   gdb_printf (":\n");
   gdb_printf (" %s = ", pc_regname);
-  if (frame_pc_p)
+  if (frame_pc)
     gdb_puts (paddress (gdbarch, get_frame_pc (fi)));
   else
     fputs_styled ("<unavailable>", metadata_style.style (), gdb_stdout);
@@ -2337,9 +2335,9 @@ print_frame_local_vars (const frame_info_ptr &frame,
 {
   struct print_variable_and_value_data cb_data;
   const struct block *block;
-  CORE_ADDR pc;
+  std::optional<CORE_ADDR> pc;
 
-  if (!get_frame_pc_if_available (frame, &pc))
+  if (!(pc = get_frame_pc_if_available (frame)))
     {
       if (!quiet)
 	gdb_printf (stream,
@@ -2499,11 +2497,11 @@ print_frame_arg_vars (const frame_info_ptr &frame,
 {
   struct print_variable_and_value_data cb_data;
   struct symbol *func;
-  CORE_ADDR pc;
+  std::optional<CORE_ADDR> pc;
   std::optional<compiled_regex> preg;
   std::optional<compiled_regex> treg;
 
-  if (!get_frame_pc_if_available (frame, &pc))
+  if (!(pc = get_frame_pc_if_available (frame)))
     {
       if (!quiet)
 	gdb_printf (stream,
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index c0f1eae3343..89f741466a6 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -208,16 +208,16 @@ set_tracepoint_num (int num)
 static void
 set_traceframe_context (const frame_info_ptr &trace_frame)
 {
-  CORE_ADDR trace_pc;
+  std::optional<CORE_ADDR> trace_pc;
   struct symbol *traceframe_fun;
   symtab_and_line traceframe_sal;
 
   /* Save as globals for internal use.  */
   if (trace_frame != NULL
-      && get_frame_pc_if_available (trace_frame, &trace_pc))
+      && (trace_pc = get_frame_pc_if_available (trace_frame)))
     {
-      traceframe_sal = find_pc_line (trace_pc, 0);
-      traceframe_fun = find_pc_function (trace_pc);
+      traceframe_sal = find_pc_line (*trace_pc, 0);
+      traceframe_fun = find_pc_function (*trace_pc);
 
       /* Save linenumber as "$trace_line", a debugger variable visible to
 	 users.  */
diff --git a/gdb/tui/tui-status.c b/gdb/tui/tui-status.c
index c2d38734721..d028f45b814 100644
--- a/gdb/tui/tui-status.c
+++ b/gdb/tui/tui-status.c
@@ -271,10 +271,14 @@ tui_show_frame_info (const frame_info_ptr &fi)
       symtab_and_line sal = find_frame_sal (fi);
 
       const char *func_name;
+      std::optional<CORE_ADDR> tmp_pc = get_frame_pc_if_available (fi);
       /* find_frame_sal does not always set PC, but we want to ensure
 	 that it is available in the SAL.  */
-      if (get_frame_pc_if_available (fi, &sal.pc))
-	func_name = tui_get_function_from_frame (fi);
+      if (tmp_pc)
+	{
+	  sal.pc = *tmp_pc;
+	  func_name = tui_get_function_from_frame (fi);
+	}
       else
 	func_name = _("<unavailable>");
 
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index a545c4870e1..b24a2e93f1e 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -461,7 +461,9 @@ tui_source_window_base::rerender ()
 
       /* find_frame_sal does not always set SAL.PC, but we want to ensure
 	 that it is available in the SAL before updating the window.  */
-      get_frame_pc_if_available (frame, &sal.pc);
+      std::optional<CORE_ADDR> tmp_pc = get_frame_pc_if_available (frame);
+      if (tmp_pc)
+	sal.pc = *tmp_pc;
 
       maybe_update (get_frame_arch (frame), sal);
       update_exec_info (false);

base-commit: 96d90166e847db11a901d435583658663224d1bc
-- 
2.50.1

Re: [PATCH] gdb: modernize get_frame_pc_if_available

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:

Guinevere> Now that GDB supports C++17 features, this indirection of a pointer and
Guinevere> returning boolean is unnecessary, since the function can return an
Guinevere> optional, and code that calls it can check if the optional contains a
Guinevere> value.

Nice change, thanks.

Guinevere> -      && frame_pc_p
Guinevere> +      && frame_pc

I think gdb normally prefers the explicit check of has_value rather than
relying on the conversion to bool.

Tom

Re: [PATCH] gdb: modernize get_frame_pc_if_available

[Guinevere Larsen]

On 8/13/25 3:21 PM, Tom Tromey wrote:
>>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:
> Guinevere> Now that GDB supports C++17 features, this indirection of a pointer and
> Guinevere> returning boolean is unnecessary, since the function can return an
> Guinevere> optional, and code that calls it can check if the optional contains a
> Guinevere> value.
>
> Nice change, thanks.
>
> Guinevere> -      && frame_pc_p
> Guinevere> +      && frame_pc
>
> I think gdb normally prefers the explicit check of has_value rather than
> relying on the conversion to bool.
Oh we do? I thought because the conversion was defined explicitly in the 
library, it would be fine, but I can update it, no problem.
>
> Tom
>

-- 
Cheers,
Guinevere Larsen
She/it

[PATCH v2] gdb: modernize get_frame_pc_if_available

[Guinevere Larsen]

The convenience function get_frame_pc_if_available would take a pointer
to a variable that should be set if available, and would return a
boolean indicating whether that action was successful or not.

Now that GDB supports C++17 features, this indirection of a pointer and
returning boolean is unnecessary, since the function can return an
optional, and code that calls it can check if the optional contains a
value.

This commit makes that modernization. It should have no visible
effects.
---
 gdb/frame.c                      | 32 +++++++++++++--------------
 gdb/frame.h                      |  3 ++-
 gdb/macroscope.c                 |  6 ++---
 gdb/printcmd.c                   |  4 ++--
 gdb/python/py-finishbreakpoint.c |  6 ++---
 gdb/stack.c                      | 38 +++++++++++++++-----------------
 gdb/tracepoint.c                 |  8 +++----
 gdb/tui/tui-status.c             |  8 +++++--
 gdb/tui/tui-winsource.c          |  4 +++-
 9 files changed, 56 insertions(+), 53 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index fc4cbca0bd2..50223f405e3 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2663,15 +2663,14 @@ get_prev_frame (const frame_info_ptr &this_frame)
 {
   FRAME_SCOPED_DEBUG_ENTER_EXIT;
 
-  CORE_ADDR frame_pc;
-  int frame_pc_p;
+  std::optional<CORE_ADDR> frame_pc;
 
   /* There is always a frame.  If this assertion fails, suspect that
      something should be calling get_selected_frame() or
      get_current_frame().  */
   gdb_assert (this_frame != NULL);
 
-  frame_pc_p = get_frame_pc_if_available (this_frame, &frame_pc);
+  frame_pc = get_frame_pc_if_available (this_frame);
 
   /* tausq/2004-12-07: Dummy frames are skipped because it doesn't make much
      sense to stop unwinding at a dummy frame.  One place where a dummy
@@ -2686,7 +2685,7 @@ get_prev_frame (const frame_info_ptr &this_frame)
   if (this_frame->level >= 0
       && get_frame_type (this_frame) == NORMAL_FRAME
       && !user_set_backtrace_options.backtrace_past_main
-      && frame_pc_p
+      && frame_pc.has_value ()
       && inside_main_func (this_frame))
     /* Don't unwind past main().  Note, this is done _before_ the
        frame has been marked as previously unwound.  That way if the
@@ -2733,7 +2732,7 @@ get_prev_frame (const frame_info_ptr &this_frame)
   if (this_frame->level >= 0
       && get_frame_type (this_frame) == NORMAL_FRAME
       && !user_set_backtrace_options.backtrace_past_entry
-      && frame_pc_p
+      && frame_pc.has_value ()
       && inside_entry_func (this_frame))
     {
       frame_debug_got_null_frame (this_frame, "inside entry func");
@@ -2747,7 +2746,7 @@ get_prev_frame (const frame_info_ptr &this_frame)
       && (get_frame_type (this_frame) == NORMAL_FRAME
 	  || get_frame_type (this_frame) == INLINE_FRAME)
       && get_frame_type (get_next_frame (this_frame)) == NORMAL_FRAME
-      && frame_pc_p && frame_pc == 0)
+      && frame_pc.has_value () && *frame_pc == 0)
     {
       frame_debug_got_null_frame (this_frame, "zero PC");
       return NULL;
@@ -2763,25 +2762,24 @@ get_frame_pc (const frame_info_ptr &frame)
   return frame_unwind_pc (frame_info_ptr (frame->next));
 }
 
-bool
-get_frame_pc_if_available (const frame_info_ptr &frame, CORE_ADDR *pc)
+std::optional<CORE_ADDR>
+get_frame_pc_if_available (const frame_info_ptr &frame)
 {
+  std::optional<CORE_ADDR> pc;
 
   gdb_assert (frame->next != NULL);
 
   try
     {
-      *pc = frame_unwind_pc (frame_info_ptr (frame->next));
+      pc = frame_unwind_pc (frame_info_ptr (frame->next));
     }
   catch (const gdb_exception_error &ex)
     {
-      if (ex.error == NOT_AVAILABLE_ERROR)
-	return false;
-      else
+      if (ex.error != NOT_AVAILABLE_ERROR)
 	throw;
     }
 
-  return true;
+  return pc;
 }
 
 /* Return an address that falls within THIS_FRAME's code block.  */
@@ -2870,7 +2868,7 @@ find_frame_sal (const frame_info_ptr &frame)
 {
   frame_info_ptr next_frame;
   int notcurrent;
-  CORE_ADDR pc;
+  std::optional<CORE_ADDR> pc;
 
   if (frame_inlined_callees (frame) > 0)
     {
@@ -2914,11 +2912,11 @@ find_frame_sal (const frame_info_ptr &frame)
      PC and such a PC indicates the current (rather than next)
      instruction/line, consequently, for such cases, want to get the
      line containing fi->pc.  */
-  if (!get_frame_pc_if_available (frame, &pc))
+  if (!(pc = get_frame_pc_if_available (frame)))
     return {};
 
-  notcurrent = (pc != get_frame_address_in_block (frame));
-  return find_pc_line (pc, notcurrent);
+  notcurrent = (*pc != get_frame_address_in_block (frame));
+  return find_pc_line (*pc, notcurrent);
 }
 
 /* Per "frame.h", return the ``address'' of the frame.  Code should
diff --git a/gdb/frame.h b/gdb/frame.h
index b240662bd73..99a79835efa 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -504,7 +504,8 @@ extern CORE_ADDR get_frame_pc (const frame_info_ptr &);
 /* Same as get_frame_pc, but return a boolean indication of whether
    the PC is actually available, instead of throwing an error.  */
 
-extern bool get_frame_pc_if_available (const frame_info_ptr &frame, CORE_ADDR *pc);
+extern std::optional<CORE_ADDR> get_frame_pc_if_available
+  (const frame_info_ptr &frame);
 
 /* An address (not necessarily aligned to an instruction boundary)
    that falls within THIS frame's code block.
diff --git a/gdb/macroscope.c b/gdb/macroscope.c
index 7aa0784866a..a8b78c9906c 100644
--- a/gdb/macroscope.c
+++ b/gdb/macroscope.c
@@ -97,12 +97,12 @@ default_macro_scope ()
 {
   struct symtab_and_line sal;
   frame_info_ptr frame;
-  CORE_ADDR pc;
+  std::optional<CORE_ADDR> pc;
 
   /* If there's a selected frame, use its PC.  */
   frame = deprecated_safe_get_selected_frame ();
-  if (frame && get_frame_pc_if_available (frame, &pc))
-    sal = find_pc_line (pc, 0);
+  if (frame && (pc = get_frame_pc_if_available (frame)))
+    sal = find_pc_line (*pc, 0);
 
   /* Fall back to the current listing position.  */
   else
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index d1cf72d371d..c6d7075e89b 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -753,10 +753,10 @@ pc_prefix (CORE_ADDR addr)
   if (has_stack_frames ())
     {
       frame_info_ptr frame;
-      CORE_ADDR pc;
+      std::optional<CORE_ADDR> pc;
 
       frame = get_selected_frame (NULL);
-      if (get_frame_pc_if_available (frame, &pc) && pc == addr)
+      if ((pc = get_frame_pc_if_available (frame)) && *pc == addr)
 	return "=> ";
     }
   return "   ";
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 0ea629f08bd..70e1684524d 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -175,7 +175,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
   struct frame_id frame_id;
   PyObject *internal = NULL;
   int internal_bp = 0;
-  CORE_ADDR pc;
+  std::optional<CORE_ADDR> pc;
 
   if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "|OO", keywords,
 					&frame_obj, &internal))
@@ -249,9 +249,9 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 
   try
     {
-      if (get_frame_pc_if_available (frame, &pc))
+      if ((pc = get_frame_pc_if_available (frame)))
 	{
-	  struct symbol *function = find_pc_function (pc);
+	  struct symbol *function = find_pc_function (*pc);
 	  if (function != nullptr)
 	    {
 	      struct type *ret_type =
diff --git a/gdb/stack.c b/gdb/stack.c
index e6335669531..e96c0a7c4e6 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1165,10 +1165,10 @@ do_print_frame_info (struct ui_out *uiout, const frame_print_options &fp_opts,
 
   if (set_current_sal)
     {
-      CORE_ADDR pc;
+      std::optional<CORE_ADDR> pc;
 
-      if (get_frame_pc_if_available (frame, &pc))
-	last_displayed_symtab_info.set (sal.pspace, pc, sal.symtab, sal.line);
+      if ((pc = get_frame_pc_if_available (frame)))
+	last_displayed_symtab_info.set (sal.pspace, *pc, sal.symtab, sal.line);
       else
 	last_displayed_symtab_info.invalidate ();
     }
@@ -1325,16 +1325,15 @@ print_frame (struct ui_out *uiout,
   enum language funlang = language_unknown;
   struct value_print_options opts;
   struct symbol *func;
-  CORE_ADDR pc = 0;
-  int pc_p;
+  std::optional <CORE_ADDR> pc;
 
-  pc_p = get_frame_pc_if_available (frame, &pc);
+  pc = get_frame_pc_if_available (frame);
 
   gdb::unique_xmalloc_ptr<char> funname
     = find_frame_funname (frame, &funlang, &func);
 
   annotate_frame_begin (print_level ? frame_relative_level (frame) : 0,
-			gdbarch, pc);
+			gdbarch, (pc.has_value ()) ? *pc : 0);
 
   {
     ui_out_emit_tuple tuple_emitter (uiout, "frame");
@@ -1352,8 +1351,8 @@ print_frame (struct ui_out *uiout,
 	  || print_what == LOC_AND_ADDRESS)
 	{
 	  annotate_frame_address ();
-	  if (pc_p)
-	    print_pc (uiout, gdbarch, frame, pc);
+	  if (pc.has_value ())
+	    print_pc (uiout, gdbarch, frame, *pc);
 	  else
 	    uiout->field_string ("addr", "<unavailable>",
 				 metadata_style.style ());
@@ -1422,7 +1421,7 @@ print_frame (struct ui_out *uiout,
       }
 
     if (print_what != SHORT_LOCATION
-	&& pc_p && (funname == NULL || sal.symtab == NULL))
+	&& pc.has_value () && (funname == NULL || sal.symtab == NULL))
       {
 	const char *lib
 	  = solib_name_from_address (get_frame_program_space (frame),
@@ -1481,8 +1480,7 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
   enum language funlang = language_unknown;
   const char *pc_regname;
   struct gdbarch *gdbarch;
-  CORE_ADDR frame_pc;
-  int frame_pc_p;
+  std::optional<CORE_ADDR> frame_pc;
   /* Initialize it to avoid "may be used uninitialized" warning.  */
   CORE_ADDR caller_pc = 0;
   int caller_pc_p = 0;
@@ -1503,7 +1501,7 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
        get_frame_pc().  */
     pc_regname = "pc";
 
-  frame_pc_p = get_frame_pc_if_available (fi, &frame_pc);
+  frame_pc = get_frame_pc_if_available (fi);
   func = get_frame_function (fi);
   symtab_and_line sal = find_frame_sal (fi);
   s = sal.symtab;
@@ -1525,9 +1523,9 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
 	    funname = func_only.get ();
 	}
     }
-  else if (frame_pc_p)
+  else if (frame_pc.has_value ())
     {
-      bound_minimal_symbol msymbol = lookup_minimal_symbol_by_pc (frame_pc);
+      bound_minimal_symbol msymbol = lookup_minimal_symbol_by_pc (*frame_pc);
       if (msymbol.minsym != NULL)
 	{
 	  funname = msymbol.minsym->print_name ();
@@ -1548,7 +1546,7 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
   gdb_puts (paddress (gdbarch, get_frame_base (fi)));
   gdb_printf (":\n");
   gdb_printf (" %s = ", pc_regname);
-  if (frame_pc_p)
+  if (frame_pc.has_value ())
     gdb_puts (paddress (gdbarch, get_frame_pc (fi)));
   else
     fputs_styled ("<unavailable>", metadata_style.style (), gdb_stdout);
@@ -2337,9 +2335,9 @@ print_frame_local_vars (const frame_info_ptr &frame,
 {
   struct print_variable_and_value_data cb_data;
   const struct block *block;
-  CORE_ADDR pc;
+  std::optional<CORE_ADDR> pc;
 
-  if (!get_frame_pc_if_available (frame, &pc))
+  if (!(pc = get_frame_pc_if_available (frame)))
     {
       if (!quiet)
 	gdb_printf (stream,
@@ -2499,11 +2497,11 @@ print_frame_arg_vars (const frame_info_ptr &frame,
 {
   struct print_variable_and_value_data cb_data;
   struct symbol *func;
-  CORE_ADDR pc;
+  std::optional<CORE_ADDR> pc;
   std::optional<compiled_regex> preg;
   std::optional<compiled_regex> treg;
 
-  if (!get_frame_pc_if_available (frame, &pc))
+  if (!(pc = get_frame_pc_if_available (frame)))
     {
       if (!quiet)
 	gdb_printf (stream,
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index c0f1eae3343..89f741466a6 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -208,16 +208,16 @@ set_tracepoint_num (int num)
 static void
 set_traceframe_context (const frame_info_ptr &trace_frame)
 {
-  CORE_ADDR trace_pc;
+  std::optional<CORE_ADDR> trace_pc;
   struct symbol *traceframe_fun;
   symtab_and_line traceframe_sal;
 
   /* Save as globals for internal use.  */
   if (trace_frame != NULL
-      && get_frame_pc_if_available (trace_frame, &trace_pc))
+      && (trace_pc = get_frame_pc_if_available (trace_frame)))
     {
-      traceframe_sal = find_pc_line (trace_pc, 0);
-      traceframe_fun = find_pc_function (trace_pc);
+      traceframe_sal = find_pc_line (*trace_pc, 0);
+      traceframe_fun = find_pc_function (*trace_pc);
 
       /* Save linenumber as "$trace_line", a debugger variable visible to
 	 users.  */
diff --git a/gdb/tui/tui-status.c b/gdb/tui/tui-status.c
index c2d38734721..1e09975eab5 100644
--- a/gdb/tui/tui-status.c
+++ b/gdb/tui/tui-status.c
@@ -271,10 +271,14 @@ tui_show_frame_info (const frame_info_ptr &fi)
       symtab_and_line sal = find_frame_sal (fi);
 
       const char *func_name;
+      std::optional<CORE_ADDR> tmp_pc = get_frame_pc_if_available (fi);
       /* find_frame_sal does not always set PC, but we want to ensure
 	 that it is available in the SAL.  */
-      if (get_frame_pc_if_available (fi, &sal.pc))
-	func_name = tui_get_function_from_frame (fi);
+      if (tmp_pc.has_value ())
+	{
+	  sal.pc = *tmp_pc;
+	  func_name = tui_get_function_from_frame (fi);
+	}
       else
 	func_name = _("<unavailable>");
 
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index a545c4870e1..618d72bbfe2 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -461,7 +461,9 @@ tui_source_window_base::rerender ()
 
       /* find_frame_sal does not always set SAL.PC, but we want to ensure
 	 that it is available in the SAL before updating the window.  */
-      get_frame_pc_if_available (frame, &sal.pc);
+      std::optional<CORE_ADDR> tmp_pc = get_frame_pc_if_available (frame);
+      if (tmp_pc.has_value ())
+	sal.pc = *tmp_pc;
 
       maybe_update (get_frame_arch (frame), sal);
       update_exec_info (false);

base-commit: 96d90166e847db11a901d435583658663224d1bc
-- 
2.50.1

Re: [PATCH v2] gdb: modernize get_frame_pc_if_available

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:

Guinevere> The convenience function get_frame_pc_if_available would take a pointer
Guinevere> to a variable that should be set if available, and would return a
Guinevere> boolean indicating whether that action was successful or not.

Thanks.

Guinevere> -  if (!get_frame_pc_if_available (frame, &pc))
Guinevere> +  if (!(pc = get_frame_pc_if_available (frame)))

This is kind of cheating but in most spots in the patch it is cleaner
than the alternative.  So I'm inclined to let it slide.

Guinevere>    annotate_frame_begin (print_level ? frame_relative_level (frame) : 0,
Guinevere> -			gdbarch, pc);
Guinevere> +			gdbarch, (pc.has_value ()) ? *pc : 0);

I think this can be written: pc.value_or (0).
Ok with that change.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH v2] gdb: modernize get_frame_pc_if_available

[Guinevere Larsen]

On 8/14/25 1:08 PM, Tom Tromey wrote:
>>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:
> Guinevere> The convenience function get_frame_pc_if_available would take a pointer
> Guinevere> to a variable that should be set if available, and would return a
> Guinevere> boolean indicating whether that action was successful or not.
>
> Thanks.
>
> Guinevere> -  if (!get_frame_pc_if_available (frame, &pc))
> Guinevere> +  if (!(pc = get_frame_pc_if_available (frame)))
>
> This is kind of cheating but in most spots in the patch it is cleaner
> than the alternative.  So I'm inclined to let it slide.
>
> Guinevere>    annotate_frame_begin (print_level ? frame_relative_level (frame) : 0,
> Guinevere> -			gdbarch, pc);
> Guinevere> +			gdbarch, (pc.has_value ()) ? *pc : 0);
>
> I think this can be written: pc.value_or (0).
> Ok with that change.
>
> Approved-By: Tom Tromey <tom@tromey.com>
>
> Tom
>
Thanks for the quick review! fixed and pushed!

-- 
Cheers,
Guinevere Larsen
She/it