[RFA] New dwarf2_attribute_true_p function in dwarf2read.c (take 2)

[RFA] New dwarf2_attribute_true_p function in dwarf2read.c (take 2)

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 617 bytes --]

Hello,

based on some comments made by Daniel in:

    http://sources.redhat.com/ml/gdb-patches/2004-04/msg00352.html

here is a new version of the patch. This time, I only check the flag
for the DW_AT_declaration attribute. I also took this opportunity to
rework a bit the comments. It sounds like better English to me, this
time (any suggestion always welcome, btw).

2004-04-16  Joel Brobecker  <brobecker@gnat.com>

        * dwarf2read.c (dwarf2_attribute_true_p): New function.
        (die_is_declaration): Use the function above. Add some comments.

Tested on x86-linux, no regression.

OK to apply?
-- 
Joel

[-- Attachment #2: true_p.diff --]
[-- Type: text/plain, Size: 2024 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.146
diff -u -p -r1.146 dwarf2read.c
--- dwarf2read.c	16 Apr 2004 16:12:52 -0000	1.146
+++ dwarf2read.c	16 Apr 2004 21:11:36 -0000
@@ -732,6 +732,9 @@ static void set_cu_language (unsigned in
 static struct attribute *dwarf2_attr (struct die_info *, unsigned int,
 				      struct dwarf2_cu *);
 
+static int dwarf2_attribute_true_p (struct die_info *die, unsigned name,
+                                    struct dwarf2_cu *cu);
+
 static int die_is_declaration (struct die_info *, struct dwarf2_cu *cu);
 
 static struct die_info *die_specification (struct die_info *die,
@@ -5549,11 +5552,30 @@ dwarf2_attr (struct die_info *die, unsig
   return NULL;
 }
 
+/* Return non-zero iff the attribute NAME is defined for the given DIE,
+   and holds a non-zero value.  */
+
+static int
+dwarf2_attribute_true_p (struct die_info *die, unsigned name,
+                         struct dwarf2_cu *cu)
+{
+  struct attribute *attr = dwarf2_attr (die, name, cu);
+
+  return (attr && DW_UNSND (attr));
+}
+
 static int
 die_is_declaration (struct die_info *die, struct dwarf2_cu *cu)
 {
-  return (dwarf2_attr (die, DW_AT_declaration, cu)
-	  && ! dwarf2_attr (die, DW_AT_specification, cu));
+  /* A DIE is a declaration if it has a DW_AT_declaration attribute
+     which value is non-zero.  However, we have to be careful with
+     DIEs having a DW_AT_specification attribute, because dwarf2_attr()
+     (via dwarf2_attribute_true_p) follows this attribute.  So we may
+     end up accidently finding a declaration attribute that belongs
+     to a different DIE referenced by the specification attribute,
+     even though the given DIE does not have a declaration attribute.  */
+  return (dwarf2_attribute_true_p (die, DW_AT_declaration, cu)
+	  && dwarf2_attr (die, DW_AT_specification, cu) == NULL);
 }
 
 /* Return the die giving the specification for DIE, if there is

Re: [RFA] New dwarf2_attribute_true_p function in dwarf2read.c (take 2)

[Jim Blandy]

Joel Brobecker <brobecker@gnat.com> writes:

> Hello,
> 
> based on some comments made by Daniel in:
> 
>     http://sources.redhat.com/ml/gdb-patches/2004-04/msg00352.html
> 
> here is a new version of the patch. This time, I only check the flag
> for the DW_AT_declaration attribute. I also took this opportunity to
> rework a bit the comments. It sounds like better English to me, this
> time (any suggestion always welcome, btw).
> 
> 2004-04-16  Joel Brobecker  <brobecker@gnat.com>
> 
>         * dwarf2read.c (dwarf2_attribute_true_p): New function.
>         (die_is_declaration): Use the function above. Add some comments.
> 
> Tested on x86-linux, no regression.
> 
> OK to apply?

I'd like the comment for dwarf2_attribute_true_p to specifically say
that it's meant for use with attributes with DW_FORM_flag values.

(Trying to read more carefully this time...)  Looks okay, otherwise.

Re: [RFA] New dwarf2_attribute_true_p function in dwarf2read.c (take 2)

[Joel Brobecker]

> > 2004-04-16  Joel Brobecker  <brobecker@gnat.com>
> > 
> >         * dwarf2read.c (dwarf2_attribute_true_p): New function.
> >         (die_is_declaration): Use the function above. Add some comments.
> > 
> > Tested on x86-linux, no regression.
> > 
> > OK to apply?
> 
> I'd like the comment for dwarf2_attribute_true_p to specifically say
> that it's meant for use with attributes with DW_FORM_flag values.
> 
> (Trying to read more carefully this time...)  Looks okay, otherwise.

I can add a comment, or if you prefer, I can change the name to make
it more explicit. Something like "form_flag_attribute_true_p", for
instance, or "dw_form_flag_attribute_true_p".

-- 
Joel

Re: [RFA] New dwarf2_attribute_true_p function in dwarf2read.c (take 2)

[Daniel Jacobowitz]

On Mon, Apr 19, 2004 at 10:09:32AM -0700, Joel Brobecker wrote:
> > > 2004-04-16  Joel Brobecker  <brobecker@gnat.com>
> > > 
> > >         * dwarf2read.c (dwarf2_attribute_true_p): New function.
> > >         (die_is_declaration): Use the function above. Add some comments.
> > > 
> > > Tested on x86-linux, no regression.
> > > 
> > > OK to apply?
> > 
> > I'd like the comment for dwarf2_attribute_true_p to specifically say
> > that it's meant for use with attributes with DW_FORM_flag values.
> > 
> > (Trying to read more carefully this time...)  Looks okay, otherwise.
> 
> I can add a comment, or if you prefer, I can change the name to make
> it more explicit. Something like "form_flag_attribute_true_p", for
> instance, or "dw_form_flag_attribute_true_p".

How about dwarf2_flag_true_p?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] New dwarf2_attribute_true_p function in dwarf2read.c (take 2)

[Joel Brobecker]

> How about dwarf2_flag_true_p?

Sounds good to me. Jim? Others?

-- 
Joel

Re: [RFA] New dwarf2_attribute_true_p function in dwarf2read.c (take 2)

[Jim Blandy]

Joel Brobecker <brobecker@gnat.com> writes:

> > How about dwarf2_flag_true_p?
> 
> Sounds good to me. Jim? Others?

Perfect.

Re: [RFA] New dwarf2_attribute_true_p function in dwarf2read.c (take 2)

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 306 bytes --]

> Perfect.

Cool. Attached is what I ended up checkin in: function name revised,
and comment also added.

2004-04-19  Joel Brobecker  <brobecker@gnat.com>

        * dwarf2read.c (dwarf2_flag_true_p): New function.
        (die_is_declaration): Use the function above. Add some comments.

Cheers,
-- 
Joel

[-- Attachment #2: true_p.diff --]
[-- Type: text/plain, Size: 2041 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.146
diff -u -p -r1.146 dwarf2read.c
--- dwarf2read.c	16 Apr 2004 16:12:52 -0000	1.146
+++ dwarf2read.c	19 Apr 2004 18:07:56 -0000
@@ -732,6 +732,9 @@ static void set_cu_language (unsigned in
 static struct attribute *dwarf2_attr (struct die_info *, unsigned int,
 				      struct dwarf2_cu *);
 
+static int dwarf2_flag_true_p (struct die_info *die, unsigned name,
+                               struct dwarf2_cu *cu);
+
 static int die_is_declaration (struct die_info *, struct dwarf2_cu *cu);
 
 static struct die_info *die_specification (struct die_info *die,
@@ -5549,11 +5552,30 @@ dwarf2_attr (struct die_info *die, unsig
   return NULL;
 }
 
+/* Return non-zero iff the attribute NAME is defined for the given DIE,
+   and holds a non-zero value.  This function should only be used for
+   DW_FORM_flag attributes.  */
+
+static int
+dwarf2_flag_true_p (struct die_info *die, unsigned name, struct dwarf2_cu *cu)
+{
+  struct attribute *attr = dwarf2_attr (die, name, cu);
+
+  return (attr && DW_UNSND (attr));
+}
+
 static int
 die_is_declaration (struct die_info *die, struct dwarf2_cu *cu)
 {
-  return (dwarf2_attr (die, DW_AT_declaration, cu)
-	  && ! dwarf2_attr (die, DW_AT_specification, cu));
+  /* A DIE is a declaration if it has a DW_AT_declaration attribute
+     which value is non-zero.  However, we have to be careful with
+     DIEs having a DW_AT_specification attribute, because dwarf2_attr()
+     (via dwarf2_flag_true_p) follows this attribute.  So we may
+     end up accidently finding a declaration attribute that belongs
+     to a different DIE referenced by the specification attribute,
+     even though the given DIE does not have a declaration attribute.  */
+  return (dwarf2_flag_true_p (die, DW_AT_declaration, cu)
+	  && dwarf2_attr (die, DW_AT_specification, cu) == NULL);
 }
 
 /* Return the die giving the specification for DIE, if there is