diff mbox series

[10/11] Add declaration-only enums to XML reader/writer.

Message ID 20200610115940.26035-11-gprocida@google.com
State New
Headers show
Series Add incomplete enum support. | expand

Commit Message

Giuliano Procida June 10, 2020, 11:59 a.m. UTC
Serialisation seems OK.

Deserialisation quite likely needs declaration/definition resolution
as there is logic for this for class types.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-reader.cc |  4 ++++
 src/abg-writer.cc | 17 +++++++++++++++++
 2 files changed, 21 insertions(+)

Comments

Dodji Seketeli July 2, 2020, 1:55 p.m. UTC | #1
Giuliano Procida <gprocida@google.com> a écrit:

> Serialisation seems OK.
>
> Deserialisation quite likely needs declaration/definition resolution
> as there is logic for this for class types.

Just for the record, I think that the deserialization doesn't need the
declaration/definition resolution logic, because the resolution happened
already when we analysed the DWARF.  The subsequent serialized abixml
doesn't have improperly resolved declarations anymore.

So I am adding ChangeLog to this patch then, and I think it should be
good to go.  I'll reply later to this patch with the new one when I am
done.

Thanks.

[...]

Cheers,
Giuliano Procida July 2, 2020, 3:09 p.m. UTC | #2
Hi Dodji.

On Thu, 2 Jul 2020 at 14:55, Dodji Seketeli <dodji@seketeli.org> wrote:
>
> Giuliano Procida <gprocida@google.com> a écrit:
>
> > Serialisation seems OK.
> >
> > Deserialisation quite likely needs declaration/definition resolution
> > as there is logic for this for class types.
>
> Just for the record, I think that the deserialization doesn't need the
> declaration/definition resolution logic, because the resolution happened
> already when we analysed the DWARF.  The subsequent serialized abixml
> doesn't have improperly resolved declarations anymore.

That's good news. I wasn't sure what to make of the existing
resolution logic in the reader code. Does it relate to an older format
of the XML?

Regards,
Giuliano.

> So I am adding ChangeLog to this patch then, and I think it should be
> good to go.  I'll reply later to this patch with the new one when I am
> done.
>
> Thanks.
>
> [...]
>
> Cheers,
>
> --
>                 Dodji
Dodji Seketeli July 6, 2020, 11:05 a.m. UTC | #3
Giuliano Procida <gprocida@google.com> a écrit:

[...]

> That's good news. I wasn't sure what to make of the existing
> resolution logic in the reader code. Does it relate to an older format
> of the XML?

Yes it does.  At the time, there was no resolution done in the dwarf
reader.  So it was leaking through the abixml.

Cheers,
Dodji Seketeli July 6, 2020, 11:31 a.m. UTC | #4
Giuliano Procida <gprocida@google.com> a écrit:

> Serialisation seems OK.
>
> Deserialisation quite likely needs declaration/definition resolution
> as there is logic for this for class types.

I have amended this patch to make it work in the context of the changes
I have done on the earlier patches of the series.  Namely, the
decl-only-ness property can now show up on any declaration, not just on
classes, unions and enums.  I have also added a ChangeLog in the commit
log.

I had to update a test reference output to reflect the new format.

The amended patch below is also stacked up in the dodji/incomp-enums
branch, browsable at
https://sourceware.org/git/?p=libabigail.git;a=shortlog;h=refs/heads/dodji/incomp-enums.

Cheers,

From 1469056ef52e5c2f44b276b5f058496a3416a716 Mon Sep 17 00:00:00 2001
From: Giuliano Procida <gprocida@google.com>
Date: Wed, 10 Jun 2020 12:59:39 +0100
Subject: [PATCH 3/4] Add declaration-only enums to XML reader/writer.

	* src/abg-reader.cc (build_enum_type_decl): Detect a
	declaration-only enum and flag it as such.
	(build_type_decl): Support reading the "is-declaration" attribute.
	(build_class_decl): Adjust.
	* src/abg-writer.cc (write_is_declaration_only): Renamed
	write_class_or_union_is_declaration_only into this.
	(write_enum_is_declaration_only): Remove.
	(write_type_decl, write_enum_type_decl)
	(write_class_decl_opening_tag, write_union_decl_opening_tag): Use
	write_is_declaration_only.
	* tests/data/test-read-dwarf/PR22122-libftdc.so.abi: Adjust.

Signed-off-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 src/abg-reader.cc                                 | 13 ++++++--
 src/abg-writer.cc                                 | 22 +++++++-------
 tests/data/test-read-dwarf/PR22122-libftdc.so.abi | 36 +++++++++++------------
 3 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index eb74659..6e37f7c 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -3457,6 +3457,9 @@ build_type_decl(read_context&		ctxt,
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "alignment-in-bits"))
     alignment_in_bits = atoi(CHAR_STR(s));
 
+  bool is_decl_only = false;
+  read_is_declaration_only(node, is_decl_only);
+
   location loc;
   read_location(ctxt, node, loc);
 
@@ -3479,6 +3482,7 @@ build_type_decl(read_context&		ctxt,
   type_decl_sptr decl(new type_decl(env, name, size_in_bits,
 				    alignment_in_bits, loc));
   decl->set_is_anonymous(is_anonymous);
+  decl->set_is_declaration_only(is_decl_only);
   if (ctxt.push_and_key_type_decl(decl, id, add_to_current_scope))
     {
       ctxt.map_xml_node_to_decl(node, decl);
@@ -4161,6 +4165,9 @@ build_enum_type_decl(read_context&	ctxt,
   location loc;
   read_location(ctxt, node, loc);
 
+  bool is_decl_only = false;
+  read_is_declaration_only(node, is_decl_only);
+
   bool is_anonymous = false;
   read_is_anonymous(node, is_anonymous);
 
@@ -4221,6 +4228,7 @@ build_enum_type_decl(read_context&	ctxt,
 					   enums, linkage_name));
   t->set_is_anonymous(is_anonymous);
   t->set_is_artificial(is_artificial);
+  t->set_is_declaration_only(is_decl_only); // TODO: more to do here!
   if (ctxt.push_and_key_type_decl(t, id, add_to_current_scope))
     {
       ctxt.map_xml_node_to_decl(node, t);
@@ -4488,8 +4496,7 @@ build_class_decl(read_context&		ctxt,
 
   if (!def_id.empty())
     {
-      class_decl_sptr d =
-	dynamic_pointer_cast<class_decl>(ctxt.get_type_decl(def_id));
+      decl_base_sptr d = is_decl(ctxt.get_type_decl(def_id));
       if (d && d->get_is_declaration_only())
 	{
 	  is_def_of_decl = true;
@@ -4507,7 +4514,7 @@ build_class_decl(read_context&		ctxt,
       // previous_declaration.
       //
       // Let's link them.
-      decl->set_earlier_declaration(previous_declaration);
+      decl->set_earlier_declaration(is_decl(previous_declaration));
       for (vector<type_base_sptr>::const_iterator i = types_ptr->begin();
 	   i != types_ptr->end();
 	   ++i)
diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index ce0bae2..bf44c76 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -876,8 +876,7 @@ static void write_elf_symbol_binding(elf_symbol::binding, ostream&);
 static bool write_elf_symbol_aliases(const elf_symbol&, ostream&);
 static bool write_elf_symbol_reference(const elf_symbol&, ostream&);
 static bool write_elf_symbol_reference(const elf_symbol_sptr, ostream&);
-static void write_class_or_union_is_declaration_only(const class_or_union_sptr&,
-						     ostream&);
+static void write_is_declaration_only(const decl_base_sptr&, ostream&);
 static void write_is_struct(const class_decl_sptr&, ostream&);
 static void write_is_anonymous(const decl_base_sptr&, ostream&);
 static void write_naming_typedef(const class_decl_sptr&, write_context&);
@@ -1777,18 +1776,16 @@ write_cdtor_const_static(bool is_ctor,
     o << " const='yes'";
 }
 
-/// Serialize the attribute "is-declaration-only", if the class or
-/// union has its 'is_declaration_only property set.
+/// Serialize the attribute "is-declaration-only", if the
+/// decl_base_sptr has its 'is_declaration_only property set.
 ///
-/// @param t the pointer to instance of @ref class_or_union to
-/// consider.
+/// @param t the pointer to instance of @ref decl_base to consider.
 ///
 /// @param o the output stream to serialize to.
 static void
-write_class_or_union_is_declaration_only(const class_or_union_sptr& t,
-					 ostream& o)
+write_is_declaration_only(const decl_base_sptr& d, ostream& o)
 {
-  if (t->get_is_declaration_only())
+  if (d->get_is_declaration_only())
     o << " is-declaration-only='yes'";
 }
 
@@ -2459,6 +2456,8 @@ write_type_decl(const type_decl_sptr& d, write_context& ctxt, unsigned indent)
 
   write_size_and_alignment(d, o);
 
+  write_is_declaration_only(d, o);
+
   write_location(d, ctxt);
 
   o << " id='" << ctxt.get_id_for_type(d) << "'" <<  "/>";
@@ -2938,6 +2937,7 @@ write_enum_type_decl(const enum_type_decl_sptr& decl,
     o << " linkage-name='" << decl->get_linkage_name() << "'";
 
   write_location(decl, ctxt);
+  write_is_declaration_only(decl, o);
 
   string i = id;
   if (i.empty())
@@ -3475,7 +3475,7 @@ write_class_decl_opening_tag(const class_decl_sptr&	decl,
 
   write_location(decl, ctxt);
 
-  write_class_or_union_is_declaration_only(decl, o);
+  write_is_declaration_only(decl, o);
 
   if (decl->get_earlier_declaration())
     {
@@ -3549,7 +3549,7 @@ write_union_decl_opening_tag(const union_decl_sptr&	decl,
 
   write_location(decl, ctxt);
 
-  write_class_or_union_is_declaration_only(decl, o);
+  write_is_declaration_only(decl, o);
 
   string i = id;
   if (i.empty())
diff --git a/tests/data/test-read-dwarf/PR22122-libftdc.so.abi b/tests/data/test-read-dwarf/PR22122-libftdc.so.abi
index 28df268..7ea5341 100644
--- a/tests/data/test-read-dwarf/PR22122-libftdc.so.abi
+++ b/tests/data/test-read-dwarf/PR22122-libftdc.so.abi
@@ -270,7 +270,7 @@
     <type-decl name='long long int' size-in-bits='64' id='type-id-16'/>
     <type-decl name='long long unsigned int' size-in-bits='64' id='type-id-17'/>
     <type-decl name='sizetype' size-in-bits='64' id='type-id-5'/>
-    <type-decl name='unnamed-enum-underlying-type' is-anonymous='yes' id='type-id-18'/>
+    <type-decl name='unnamed-enum-underlying-type' is-anonymous='yes' is-declaration-only='yes' id='type-id-18'/>
     <type-decl name='unsigned char' size-in-bits='8' id='type-id-19'/>
     <type-decl name='unsigned int' size-in-bits='32' id='type-id-20'/>
     <type-decl name='unsigned long int' size-in-bits='64' id='type-id-21'/>
@@ -500,7 +500,7 @@
         <class-decl name='basic_string&lt;char, std::char_traits&lt;char&gt;, std::allocator&lt;char&gt; &gt;' size-in-bits='256' visibility='default' is-declaration-only='yes' id='type-id-92'>
 
             <member-type access='private'>
-              <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-150'>
+              <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-150'>
                 <underlying-type type-id='type-id-18'/>
               </enum-decl>
             </member-type>
@@ -969,7 +969,7 @@
           </function-decl>
         </member-function>
       </class-decl>
-      <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-172'>
+      <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-172'>
         <underlying-type type-id='type-id-18'/>
       </enum-decl>
       <typedef-decl name='memory_order' type-id='type-id-172' filepath='/usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/atomic_base.h' line='63' column='1' id='type-id-171'/>
@@ -1140,7 +1140,7 @@
       <class-decl name='__anonymous_struct__2' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-32'>
 
           <member-type access='private'>
-            <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-184'>
+            <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-184'>
               <underlying-type type-id='type-id-18'/>
             </enum-decl>
           </member-type>
@@ -3878,7 +3878,7 @@
       <class-decl name='__anonymous_struct__17' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-232'>
 
           <member-type access='private'>
-            <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-240'>
+            <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-240'>
               <underlying-type type-id='type-id-18'/>
             </enum-decl>
           </member-type>
@@ -4401,7 +4401,7 @@
       <class-decl name='__anonymous_struct__6' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-174'>
 
           <member-type access='private'>
-            <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-255'>
+            <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-255'>
               <underlying-type type-id='type-id-18'/>
             </enum-decl>
           </member-type>
@@ -5356,7 +5356,7 @@
         <class-decl name='__anonymous_struct__3' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-57'>
 
             <member-type access='private'>
-              <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-337'>
+              <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-337'>
                 <underlying-type type-id='type-id-18'/>
               </enum-decl>
             </member-type>
@@ -5409,7 +5409,7 @@
       <class-decl name='__anonymous_struct__1' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-28'>
 
           <member-type access='private'>
-            <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-338'>
+            <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-338'>
               <underlying-type type-id='type-id-18'/>
             </enum-decl>
           </member-type>
@@ -5427,7 +5427,7 @@
       <class-decl name='__anonymous_struct__8' is-anonymous='yes' naming-typedef-id='type-id-334' visibility='default' is-declaration-only='yes' id='type-id-175'>
 
           <member-type access='private'>
-            <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-339'>
+            <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-339'>
               <underlying-type type-id='type-id-18'/>
             </enum-decl>
           </member-type>
@@ -7103,14 +7103,14 @@
         <class-decl name='__anonymous_struct__2' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-32'>
 
             <member-type access='private'>
-              <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-402'>
+              <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-402'>
                 <underlying-type type-id='type-id-18'/>
               </enum-decl>
             </member-type>
         </class-decl>
       </namespace-decl>
       <namespace-decl name='FTDCBSONUtil'>
-        <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-383'>
+        <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-383'>
           <underlying-type type-id='type-id-18'/>
         </enum-decl>
       </namespace-decl>
@@ -7272,7 +7272,7 @@
       <class-decl name='__anonymous_struct__19' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-234'>
 
           <member-type access='private'>
-            <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-403'>
+            <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-403'>
               <underlying-type type-id='type-id-18'/>
             </enum-decl>
           </member-type>
@@ -7456,7 +7456,7 @@
           <class-decl name='__anonymous_struct__1' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-28'/>
         </member-type>
       </class-decl>
-      <enum-decl name='__anonymous_enum__1' is-anonymous='yes' id='type-id-426'>
+      <enum-decl name='__anonymous_enum__1' is-anonymous='yes' is-declaration-only='yes' id='type-id-426'>
         <underlying-type type-id='type-id-18'/>
       </enum-decl>
       <typedef-decl name='streamsize' type-id='type-id-163' filepath='/usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/postypes.h' line='98' column='1' id='type-id-181'/>
@@ -7905,7 +7905,7 @@
         <class-decl name='__anonymous_struct__' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-24'>
 
             <member-type access='private'>
-              <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-432'>
+              <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-432'>
                 <underlying-type type-id='type-id-18'/>
               </enum-decl>
             </member-type>
@@ -7986,7 +7986,7 @@
       <class-decl name='__anonymous_struct__11' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-186'>
 
           <member-type access='private'>
-            <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-433'>
+            <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-433'>
               <underlying-type type-id='type-id-18'/>
             </enum-decl>
           </member-type>
@@ -8680,7 +8680,7 @@
           </function-decl>
         </member-function>
       </class-decl>
-      <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-459'>
+      <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-459'>
         <underlying-type type-id='type-id-18'/>
       </enum-decl>
       <class-decl name='__anonymous_struct__14' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-229'/>
@@ -8809,7 +8809,7 @@
           </function-decl>
         </member-function>
       </class-decl>
-      <enum-decl name='__anonymous_enum__1' is-anonymous='yes' id='type-id-460'>
+      <enum-decl name='__anonymous_enum__1' is-anonymous='yes' is-declaration-only='yes' id='type-id-460'>
         <underlying-type type-id='type-id-18'/>
       </enum-decl>
       <class-decl name='__anonymous_struct__23' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-220'>
@@ -8864,7 +8864,7 @@
       <class-decl name='__anonymous_struct__26' is-anonymous='yes' visibility='default' is-declaration-only='yes' id='type-id-308'>
 
           <member-type access='private'>
-            <enum-decl name='__anonymous_enum__' is-anonymous='yes' id='type-id-461'>
+            <enum-decl name='__anonymous_enum__' is-anonymous='yes' is-declaration-only='yes' id='type-id-461'>
               <underlying-type type-id='type-id-18'/>
             </enum-decl>
           </member-type>
diff mbox series

Patch

diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index eb74659f..62eda332 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -4161,6 +4161,9 @@  build_enum_type_decl(read_context&	ctxt,
   location loc;
   read_location(ctxt, node, loc);
 
+  bool is_decl_only = false;
+  read_is_declaration_only(node, is_decl_only);
+
   bool is_anonymous = false;
   read_is_anonymous(node, is_anonymous);
 
@@ -4221,6 +4224,7 @@  build_enum_type_decl(read_context&	ctxt,
 					   enums, linkage_name));
   t->set_is_anonymous(is_anonymous);
   t->set_is_artificial(is_artificial);
+  t->set_is_declaration_only(is_decl_only); // TODO: more to do here!
   if (ctxt.push_and_key_type_decl(t, id, add_to_current_scope))
     {
       ctxt.map_xml_node_to_decl(node, t);
diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index bafa3024..38320867 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -840,6 +840,8 @@  static bool write_elf_symbol_reference(const elf_symbol&, ostream&);
 static bool write_elf_symbol_reference(const elf_symbol_sptr, ostream&);
 static void write_class_or_union_is_declaration_only(const class_or_union_sptr&,
 						     ostream&);
+static void write_enum_is_declaration_only(const enum_type_decl_sptr&,
+					   ostream&);
 static void write_is_struct(const class_decl_sptr&, ostream&);
 static void write_is_anonymous(const decl_base_sptr&, ostream&);
 static void write_naming_typedef(const class_decl_sptr&, write_context&);
@@ -1754,6 +1756,20 @@  write_class_or_union_is_declaration_only(const class_or_union_sptr& t,
     o << " is-declaration-only='yes'";
 }
 
+/// Serialize the attribute "is-declaration-only", if the enum has its
+/// is_declaration_only property set.
+///
+/// @param t the pointer to instance of @ref enum_type_decl to
+/// consider.
+///
+/// @param o the output stream to serialize to.
+static void
+write_enum_is_declaration_only(const enum_type_decl_sptr& t, ostream& o)
+{
+  if (t->get_is_declaration_only())
+    o << " is-declaration-only='yes'";
+}
+
 /// Serialize the attribute "is-struct", if the current instance of
 /// class_decl is a struct.
 ///
@@ -2889,6 +2905,7 @@  write_enum_type_decl(const enum_type_decl_sptr& decl,
     o << " linkage-name='" << decl->get_linkage_name() << "'";
 
   write_location(decl, ctxt);
+  write_enum_is_declaration_only(decl, o);
 
   string i = id;
   if (i.empty())