[3/7] gdb: move frame_info_ptr method implementations to frame-info.c

Message ID 20221107155310.2590069-3-simon.marchi@polymtl.ca
State Committed
Commit 04e2ac7b2a7c5fbc0afcf151aeb8a26415ad0fac
Headers
Series [1/7] gdb: clear other.m_cached_id in frame_info_ptr's move ctor |

Commit Message

Simon Marchi Nov. 7, 2022, 3:53 p.m. UTC
  I don't see any particular reason why the implementations of the
frame_info_ptr object are in the header file.  It only seems to add some
complexity.  Since we can't include frame.h in frame-info.h, we have to
add declarations of functions defined in frame.c, in frame-info.h.  By
moving the implementations to a new frame-info.c, we can avoid that.

Change-Id: I435c828f81b8a3392c43ef018af31effddf6be9c
---
 gdb/Makefile.in  |  1 +
 gdb/frame-info.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 gdb/frame-info.h | 20 ++------------------
 gdb/frame.c      |  4 +---
 gdb/frame.h      |  4 ++++
 5 files changed, 55 insertions(+), 21 deletions(-)
 create mode 100644 gdb/frame-info.c
  

Comments

Guinevere Larsen Nov. 8, 2022, 9:55 a.m. UTC | #1
On 07/11/2022 16:53, Simon Marchi via Gdb-patches wrote:
> I don't see any particular reason why the implementations of the
> frame_info_ptr object are in the header file.  It only seems to add some
> complexity.  Since we can't include frame.h in frame-info.h, we have to
> add declarations of functions defined in frame.c, in frame-info.h.  By
> moving the implementations to a new frame-info.c, we can avoid that.

My guess is that Tom was prototyping the solution like this, and I 
caught it and sent it before finishing his train of thoughts. (That or I 
wasn't sure of how to add a new .c file, always a possibility...)

This patch makes sense and generates no build issues or regressions on 
my system, so:

Reviewed-By: Bruno Larsen <blarsen@redhat.com>
  
Tom Tromey Nov. 8, 2022, 5:39 p.m. UTC | #2
>>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

Bruno> On 07/11/2022 16:53, Simon Marchi via Gdb-patches wrote:
>> I don't see any particular reason why the implementations of the
>> frame_info_ptr object are in the header file.  It only seems to add some
>> complexity.  Since we can't include frame.h in frame-info.h, we have to
>> add declarations of functions defined in frame.c, in frame-info.h.  By
>> moving the implementations to a new frame-info.c, we can avoid that.

Bruno> My guess is that Tom was prototyping the solution like this, and I
Bruno> caught it and sent it before finishing his train of thoughts.

I don't remember but this seems like an improvement to me.

Tom
  

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index c528ee5aa806..32dbbb9c7003 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1086,6 +1086,7 @@  COMMON_SFILES = \
 	findvar.c \
 	frame.c \
 	frame-base.c \
+	frame-info.c \
 	frame-unwind.c \
 	gcore.c \
 	gdb-demangle.c \
diff --git a/gdb/frame-info.c b/gdb/frame-info.c
new file mode 100644
index 000000000000..84791205d906
--- /dev/null
+++ b/gdb/frame-info.c
@@ -0,0 +1,47 @@ 
+/* Frame info pointer
+
+   Copyright (C) 2022 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+
+#include "frame-info.h"
+#include "frame.h"
+
+/* See frame-info-ptr.h.  */
+
+intrusive_list<frame_info_ptr> frame_info_ptr::frame_list;
+
+/* See frame-info-ptr.h.  */
+
+void
+frame_info_ptr::prepare_reinflate ()
+{
+  m_cached_id = get_frame_id (*this);
+}
+
+/* See frame-info-ptr.h.  */
+
+void
+frame_info_ptr::reinflate ()
+{
+  gdb_assert (m_cached_id != null_frame_id);
+
+  if (m_ptr == nullptr)
+    m_ptr = frame_find_by_id (m_cached_id).get ();
+  gdb_assert (m_ptr != nullptr);
+}
diff --git a/gdb/frame-info.h b/gdb/frame-info.h
index 7159f82b1962..1d2d4bdc7e68 100644
--- a/gdb/frame-info.h
+++ b/gdb/frame-info.h
@@ -25,12 +25,6 @@ 
 
 struct frame_info;
 
-/* Forward declarations of functions, needed for the frame_info_ptr
-   to work correctly.  */
-extern void reinit_frame_cache ();
-extern struct frame_id get_frame_id (frame_info_ptr);
-extern frame_info_ptr frame_find_by_id (struct frame_id id);
-
 /* A wrapper for "frame_info *".  frame_info objects are invalidated
    whenever reinit_frame_cache is called.  This class arranges to
    invalidate the pointer when appropriate.  This is done to help
@@ -136,20 +130,10 @@  class frame_info_ptr : public intrusive_list_node<frame_info_ptr>
   }
 
   /* Cache the frame_id that the pointer will use to reinflate.  */
-  void prepare_reinflate ()
-  {
-    m_cached_id = get_frame_id (*this);
-  }
+  void prepare_reinflate ();
 
   /* Use the cached frame_id to reinflate the pointer.  */
-  void reinflate ()
-  {
-    gdb_assert (m_cached_id != null_frame_id);
-
-    if (m_ptr == nullptr)
-      m_ptr = frame_find_by_id (m_cached_id).get ();
-    gdb_assert (m_ptr != nullptr);
-  }
+  void reinflate ();
 
 private:
 
diff --git a/gdb/frame.c b/gdb/frame.c
index 5cf9186e43d7..997fda77d019 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -19,6 +19,7 @@ 
 
 #include "defs.h"
 #include "frame.h"
+#include "frame-info.h"
 #include "target.h"
 #include "value.h"
 #include "inferior.h"	/* for inferior_ptid */
@@ -56,9 +57,6 @@  static frame_info *sentinel_frame;
 /* Number of calls to reinit_frame_cache.  */
 static unsigned int frame_cache_generation = 0;
 
-/* See frame-info.h.  */
-intrusive_list<frame_info_ptr> frame_info_ptr::frame_list;
-
 /* See frame.h.  */
 
 unsigned int
diff --git a/gdb/frame.h b/gdb/frame.h
index f61ea63c290b..5f7dcb69ee90 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -287,6 +287,10 @@  extern frame_info_ptr get_next_frame_sentinel_okay (frame_info_ptr);
    frame.  */
 extern frame_info_ptr get_prev_frame_always (frame_info_ptr);
 
+/* Given a frame's ID, relocate the frame.  Returns NULL if the frame
+   is not found.  */
+extern frame_info_ptr frame_find_by_id (frame_id id);
+
 /* Base attributes of a frame: */
 
 /* The frame's `resume' address.  Where the program will resume in