public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
From: Giuliano Procida <gprocida@google.com>
To: libabigail@sourceware.org
Cc: dodji@seketeli.org, kernel-team@android.com, gprocida@google.com,
	 maennich@google.com
Subject: [PATCH] abg-reader.cc: track WIP types by pointer not name
Date: Mon, 22 Jun 2020 17:06:19 +0100	[thread overview]
Message-ID: <20200622160619.4154-1-gprocida@google.com> (raw)

When reading ABI XML files, the reader needs to construct types
progressively as any type may depend on other types and even on
itself. Such work-in-progress types are tracked explicitly.

The storage used for this is a map from (external, qualified) type
name to a count of how many times the type (name) has been seen.

However, function type names are invariably stored as "void ()" as
they are incomplete at the point they are added to the map. When the
reader later attempts to remove the marking they have their proper,
different names. In short, the code doesn't do what it's supposed to.

This commit changes the map key from string to const type_base*.
Equality on type_base_sptr has been defined to be a deep comparison so
storing those wouldn't be quite right.

Note that this removes some of the call paths that result in incorrect
(external) type names being cached (regardless of whether they are
actually used).

	* src/abg-reader.cc (xml_reader::m_wip_types_map): Change type
	to unordered_map<const type_base*, size_t>.
	(xml_reader::mark_type_as_wip): Just increment count of type
	pointer.
	(xml_reader::unmark_type_as_wip): Add assertion that type
	pointer is present. Decrement type pointer count and remove if
	zero.
	(xml_reader::is_wip_type): Test if type pointer in map.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-reader.cc | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index eb74659f..fe82f4ec 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -119,7 +119,7 @@ private:
   unordered_map<string, vector<type_base_sptr> >	m_types_map;
   unordered_map<string, shared_ptr<function_tdecl> >	m_fn_tmpl_map;
   unordered_map<string, shared_ptr<class_tdecl> >	m_class_tmpl_map;
-  unordered_map<string, size_t>			m_wip_types_map;
+  abg_compat::unordered_map<const type_base*, size_t>	m_wip_types_map;
   vector<type_base_sptr>				m_types_to_canonicalize;
   string_xml_node_map					m_id_xml_node_map;
   xml_node_decl_base_sptr_map				m_xml_node_decl_map;
@@ -534,12 +534,7 @@ public:
   {
     if (!t)
       return;
-    string qname = get_type_name(t, /*qualified=*/true);
-    unordered_map<string, size_t>::iterator it = m_wip_types_map.find(qname);
-    if (it == m_wip_types_map.end())
-      m_wip_types_map[qname] = 1;
-    else
-      ++it->second;
+    ++(m_wip_types_map[t.get()]);
   }
 
   /// Mark a given class as being *NOT* "Work In Progress" anymore;
@@ -551,13 +546,10 @@ public:
   {
     if (!t)
       return;
-
-    string qname = get_type_name(t, /*qualified=*/true);
-    unordered_map<string, size_t>::iterator it = m_wip_types_map.find(qname);
-    if (it == m_wip_types_map.end())
-      return;
-    if (it->second)
-      --it->second;
+    unordered_map<const type_base*, size_t>::iterator it =
+      m_wip_types_map.find(t.get());
+    ABG_ASSERT(it != m_wip_types_map.end() && it->second > 0);
+    --it->second;
     if (it->second == 0)
       m_wip_types_map.erase(it);
   }
@@ -571,11 +563,7 @@ public:
   {
     if (!t)
       return false;
-
-    string qname = get_type_name(t, /*qualified=*/true);
-    unordered_map<string, size_t>::const_iterator i =
-      m_wip_types_map.find(qname);
-    return i != m_wip_types_map.end();
+    return m_wip_types_map.count(t.get());
   }
 
   /// Test if two types are equal, without comparing them structurally.
-- 
2.27.0.111.gc72c7da667-goog


             reply	other threads:[~2020-06-22 16:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-22 16:06 Giuliano Procida [this message]
2020-06-22 17:25 ` [PATCH v2] " Giuliano Procida
2020-06-22 20:06   ` Matthias Maennich
2020-07-09 12:58   ` Dodji Seketeli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200622160619.4154-1-gprocida@google.com \
    --to=gprocida@google.com \
    --cc=dodji@seketeli.org \
    --cc=kernel-team@android.com \
    --cc=libabigail@sourceware.org \
    --cc=maennich@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).