Re: [PATCH 08/20] corpus: make get_unreferenced_(function|variable)_symbols use the new symtab

[Dodji Seketeli <dodji@seketeli.org>]

Hello,

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

[...]

> diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc
> index 75f897eb72a8..f9f51a360707 100644
> --- a/src/abg-corpus.cc
> +++ b/src/abg-corpus.cc

[...]

> +/// Return a list of symbols that are not referenced by any function of
> +/// corpus::get_functions().
> +///
> +/// Note that this function considers the list of function symbols to keep,
> +/// that is provided by corpus::get_sym_ids_of_fns_to_keep(). If a given
> +/// unreferenced function symbol is not in the list of functions to keep, then
> +/// that symbol is dropped and will not be part of the resulting table of
> +/// unreferenced symbol that is built.

Please use the @return tag in this comment.

> +const elf_symbols&
> +corpus::priv::get_unreferenced_function_symbols() const
> +{

[...]

> +/// Return a list of symbols that are not referenced by any variable of
> +/// corpus::get_variables().
> +///
> +/// Note that this function considers the list of variable symbols to keep,
> +/// that is provided by corpus::get_sym_ids_of_vars_to_keep(). If a given
> +/// unreferenced variable symbol is not in the list of variable to keep, then
> +/// that symbol is dropped and will not be part of the resulting table of
> +/// unreferenced symbol that is built.

Likewise.

> +const elf_symbols&
> +corpus::priv::get_unreferenced_variable_symbols() const
> +{

[...]

>  /// Getter of the set of pretty representation of types that are
>  /// reachable from public interfaces (global functions and variables).
>  ///
> @@ -1310,12 +1290,7 @@ corpus::sort_variables()
>  /// function exported by the current corpus.
>  const elf_symbols&
>  corpus::get_unreferenced_function_symbols() const
> -{
> -  if (priv_->unrefed_fun_symbols.empty()
> -      && priv_->unrefed_var_symbols.empty())
> -    priv_->build_unreferenced_symbols_tables();
> -  return priv_->unrefed_fun_symbols;
> -}
> +{ return priv_->get_unreferenced_function_symbols(); }

Please remove leading/trailing spaces after/before the curly brackets.

>  
>  /// Getter of the set of variable symbols that are not referenced by
>  /// any variable exported by the current corpus.
> @@ -1328,12 +1303,7 @@ corpus::get_unreferenced_function_symbols() const
>  /// variable exported by the current corpus.
>  const elf_symbols&
>  corpus::get_unreferenced_variable_symbols() const
> -{
> -    if (priv_->unrefed_fun_symbols.empty()
> -      && priv_->unrefed_var_symbols.empty())
> -    priv_->build_unreferenced_symbols_tables();
> -  return priv_->unrefed_var_symbols;
> -}
> +{ return priv_->get_unreferenced_variable_symbols(); }

[...]

> Make the corresponding members an implementation detail of corpus::priv.
> They get computed based on the new symtab whenever they are accessed
> first with an atomic instantiation. That simplifies the implementation
> and homogenizes the access to functions and variables. Sorting does not
> need to be done as the symtab already gives a guarantee for that.
>
>         * src/abg-corpus-priv.h (corpus::priv::unrefed_var_symbols): make
>           private, mutable and optional.
>           (corpus::unrefed_fun_symbols): Likewise.
>           (corpus::priv::get_unreferenced_function_symbols): New method declaration.
>           (corpus::priv::get_unreferenced_variable_symbols): Likewise.
>         * src/abg-corpus.cc
>           (corpus::priv::build_unreferenced_symbols_tables): Delete method.
>           (corpus::priv::get_unreferenced_function_symbols): New method implementation.
>           (corpus::priv::get_unreferenced_variable_symbols): Likewise.
>           (corpus::get_unreferenced_function_symbols): Proxy call to corpus::priv.
>           (corpus::get_unreferenced_variable_symbols): Likewise.
>
> Reviewed-by: Giuliano Procida <gprocida@google.com>
> Signed-off-by: Matthias Maennich <maennich@google.com>

OK to apply to master with the above changes once the previous patches
have been applied.

Thanks!

[...]

cheers,

-- 
		Dodji