public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] corpus: is_empty: consider actual translation unit contents
  2020-01-01  0:00 ` [PATCH 1/2] corpus: is_empty: consider actual translation unit contents Matthias Maennich via libabigail
@ 2020-01-01  0:00   ` Dodji Seketeli
  0 siblings, 0 replies; 6+ messages in thread
From: Dodji Seketeli @ 2020-01-01  0:00 UTC (permalink / raw)
  To: Matthias Maennich; +Cc: libabigail, kernel-team

Hello Matthias,

Matthias Maennich <maennich@google.com> a écrit:

[...]

> diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc
> index e37c35f5fbbb..bee87ddbb307 100644
> --- a/src/abg-corpus.cc
> +++ b/src/abg-corpus.cc
> @@ -859,7 +859,18 @@ corpus::set_architecture_name(const string& arch)
>  bool
>  corpus::is_empty() const
>  {
> -  return (priv_->members.empty()
> +  bool members_empty = true;
> +  for (translation_units::const_iterator i = priv_->members.begin(),
> +					 e = priv_->members.end();
> +       i != e; ++i)
> +    {
> +      if (!(*i)->is_empty())
> +	{
> +	  members_empty = false;
> +	  break;
> +	}
> +    }
> +  return (members_empty
>  	  && priv_->fun_symbol_map
>  	  && priv_->fun_symbol_map->empty()
>  	  && priv_->var_symbol_map

This change obviously looks good to me.  However, I think it's useful to
update the comment of the corpus::is_empty function to say that the
function tests if the corpus contains no non-empty translation unit.

[...]


> Hence, teach is_empty() to have a look at the actual translation units.
>
> 	* src/abg-corpus.cc (corpus::is_empty): consider a list of
> 	  empty members to be empty.

This is thus OK to commit to master with the the comment update above.

Thanks for working on this!

Cheers,

-- 
		Dodji

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] writer: completely skip over empty corpora
  2020-01-01  0:00 [PATCH 0/2] corpus/writer: omit empty corpora Matthias Maennich via libabigail
  2020-01-01  0:00 ` [PATCH 1/2] corpus: is_empty: consider actual translation unit contents Matthias Maennich via libabigail
  2020-01-01  0:00 ` [PATCH 0/2] corpus/writer: omit empty corpora Matthias Maennich via libabigail
@ 2020-01-01  0:00 ` Matthias Maennich via libabigail
  2020-01-01  0:00   ` Dodji Seketeli
  2 siblings, 1 reply; 6+ messages in thread
From: Matthias Maennich via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, maennich

A corpus that has no symbols contributing to the ABI surface (e.g.
because of an exhaustive suppression), will not contribute in a later
comparison via abidiff and friends. Hence, there is no need for such
entries to appear in the ABI xml representation. This patch completely
suppresses empty corpora.

	* src/abg-writer.cc (write_corpus): completely skip empty
	corpora rather than creating an empty entry for them.

Signed-off-by: Matthias Maennich <maennich@google.com>
---
 src/abg-writer.cc | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index 994a21e28066..ca10af8d5e7e 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -4378,6 +4378,9 @@ write_corpus(write_context&	ctxt,
   if (!corpus)
     return false;
 
+  if (corpus->is_empty())
+    return true;
+
   do_indent_to_level(ctxt, indent, 0);
 
   std::ostream& out = ctxt.get_ostream();
@@ -4411,12 +4414,6 @@ write_corpus(write_context&	ctxt,
 
   write_tracking_non_reachable_types(corpus, out);
 
-  if (corpus->is_empty())
-    {
-      out << "/>\n";
-      return true;
-    }
-
   out << ">\n";
 
   // Write the list of needed corpora.
-- 
2.25.0.rc1.283.g88dfdc4193-goog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] corpus: is_empty: consider actual translation unit contents
  2020-01-01  0:00 [PATCH 0/2] corpus/writer: omit empty corpora Matthias Maennich via libabigail
@ 2020-01-01  0:00 ` Matthias Maennich via libabigail
  2020-01-01  0:00   ` Dodji Seketeli
  2020-01-01  0:00 ` [PATCH 0/2] corpus/writer: omit empty corpora Matthias Maennich via libabigail
  2020-01-01  0:00 ` [PATCH 2/2] writer: completely skip over " Matthias Maennich via libabigail
  2 siblings, 1 reply; 6+ messages in thread
From: Matthias Maennich via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, maennich

A corpus with completely filtered out symbols (exhaustive whitelist),
still contains compilation units, but they are empty. A list of empty
translation units shall be considered empty as no entries need to be
considered. That is useful to skip empty corpora when writing out the
xml for them.

Hence, teach is_empty() to have a look at the actual translation units.

	* src/abg-corpus.cc (corpus::is_empty): consider a list of
	  empty members to be empty.

Signed-off-by: Matthias Maennich <maennich@google.com>
---
 src/abg-corpus.cc | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc
index e37c35f5fbbb..bee87ddbb307 100644
--- a/src/abg-corpus.cc
+++ b/src/abg-corpus.cc
@@ -859,7 +859,18 @@ corpus::set_architecture_name(const string& arch)
 bool
 corpus::is_empty() const
 {
-  return (priv_->members.empty()
+  bool members_empty = true;
+  for (translation_units::const_iterator i = priv_->members.begin(),
+					 e = priv_->members.end();
+       i != e; ++i)
+    {
+      if (!(*i)->is_empty())
+	{
+	  members_empty = false;
+	  break;
+	}
+    }
+  return (members_empty
 	  && priv_->fun_symbol_map
 	  && priv_->fun_symbol_map->empty()
 	  && priv_->var_symbol_map
-- 
2.25.0.rc1.283.g88dfdc4193-goog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] writer: completely skip over empty corpora
  2020-01-01  0:00 ` [PATCH 2/2] writer: completely skip over " Matthias Maennich via libabigail
@ 2020-01-01  0:00   ` Dodji Seketeli
  0 siblings, 0 replies; 6+ messages in thread
From: Dodji Seketeli @ 2020-01-01  0:00 UTC (permalink / raw)
  To: Matthias Maennich; +Cc: libabigail, kernel-team

Hello Matthias,

Matthias Maennich <maennich@google.com> a écrit:

[...]

> diff --git a/src/abg-writer.cc b/src/abg-writer.cc
> index 994a21e28066..ca10af8d5e7e 100644
> --- a/src/abg-writer.cc
> +++ b/src/abg-writer.cc
> @@ -4378,6 +4378,9 @@ write_corpus(write_context&	ctxt,
>    if (!corpus)
>      return false;
>  
> +  if (corpus->is_empty())
> +    return true;
> +
>    do_indent_to_level(ctxt, indent, 0);
>  
>    std::ostream& out = ctxt.get_ostream();
> @@ -4411,12 +4414,6 @@ write_corpus(write_context&	ctxt,
>  
>    write_tracking_non_reachable_types(corpus, out);
>  
> -  if (corpus->is_empty())
> -    {
> -      out << "/>\n";
> -      return true;
> -    }
> -
>    out << ">\n";

This change is OK.  It'd be useful to add a note to the comment of the
write_corpus function to say that nothing is emitted for an empty
corpus.

[...]

> A corpus that has no symbols contributing to the ABI surface (e.g.
> because of an exhaustive suppression), will not contribute in a later
> comparison via abidiff and friends. Hence, there is no need for such
> entries to appear in the ABI xml representation. This patch completely
> suppresses empty corpora.
>
> 	* src/abg-writer.cc (write_corpus): completely skip empty
> 	corpora rather than creating an empty entry for them.

OK to commit to master with the comment udpate above.

Thank you for working on this!

Cheers,

-- 
		Dodji

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] corpus/writer: omit empty corpora
  2020-01-01  0:00 [PATCH 0/2] corpus/writer: omit empty corpora Matthias Maennich via libabigail
  2020-01-01  0:00 ` [PATCH 1/2] corpus: is_empty: consider actual translation unit contents Matthias Maennich via libabigail
@ 2020-01-01  0:00 ` Matthias Maennich via libabigail
  2020-01-01  0:00 ` [PATCH 2/2] writer: completely skip over " Matthias Maennich via libabigail
  2 siblings, 0 replies; 6+ messages in thread
From: Matthias Maennich via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team

On Mon, Jan 13, 2020 at 02:44:49PM +0000, Matthias Maennich wrote:
>Empty corpora do not contribute to the ABI surface other than that they are
>present but empty and therefore not considered. Hence, drop these entries from
>the ABI XML representation.
>

I applied the series with the documentation changes mentioned.

Cheers,
Matthias


>Matthias Maennich (2):
>  corpus: is_empty: consider actual translation unit contents
>  writer: completely skip over empty corpora
>
> src/abg-corpus.cc | 13 ++++++++++++-
> src/abg-writer.cc |  9 +++------
> 2 files changed, 15 insertions(+), 7 deletions(-)
>
>-- 
>2.25.0.rc1.283.g88dfdc4193-goog
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 0/2] corpus/writer: omit empty corpora
@ 2020-01-01  0:00 Matthias Maennich via libabigail
  2020-01-01  0:00 ` [PATCH 1/2] corpus: is_empty: consider actual translation unit contents Matthias Maennich via libabigail
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Matthias Maennich via libabigail @ 2020-01-01  0:00 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, maennich

Empty corpora do not contribute to the ABI surface other than that they are
present but empty and therefore not considered. Hence, drop these entries from
the ABI XML representation.

Matthias Maennich (2):
  corpus: is_empty: consider actual translation unit contents
  writer: completely skip over empty corpora

 src/abg-corpus.cc | 13 ++++++++++++-
 src/abg-writer.cc |  9 +++------
 2 files changed, 15 insertions(+), 7 deletions(-)

-- 
2.25.0.rc1.283.g88dfdc4193-goog

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-01-20 15:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-01  0:00 [PATCH 0/2] corpus/writer: omit empty corpora Matthias Maennich via libabigail
2020-01-01  0:00 ` [PATCH 1/2] corpus: is_empty: consider actual translation unit contents Matthias Maennich via libabigail
2020-01-01  0:00   ` Dodji Seketeli
2020-01-01  0:00 ` [PATCH 0/2] corpus/writer: omit empty corpora Matthias Maennich via libabigail
2020-01-01  0:00 ` [PATCH 2/2] writer: completely skip over " Matthias Maennich via libabigail
2020-01-01  0:00   ` Dodji Seketeli

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).