diff mbox series

[v3,15/21] abg-suppression.cc: More consistent regex matching.

Message ID 20200424092132.150547-16-gprocida@google.com
State Superseded
Headers show
Series Simplify regex and suppression parsing. | expand

Commit Message

Giuliano Procida April 24, 2020, 9:21 a.m. UTC
Following previous patches, various minor inconsistencies in the
suppression regex matching code were more readily apparent.

- regex_t_sptr values are fetched both by reference and by
  value (which has a reference count mutation cost)
- there's a mixture of code styles for testing regex presence
    - regex = ...; if (regex && ...)
    - if (regex = ...) if (...)
  the latter has the advantage that the variable has a smaller scope
  and can be given a shorter name
- in some cases there are redundant tests (always true due to previous
  conditional logic)

This patch ensures all shared pointers to compiled regexes are fetched
by const reference and that the code uses the nested if style of
presence checking where possible. It simplifies some logic where
there's redundancy.

There are no behavioural changes. There may be performance
improvements.

	* src/abg-suppression.cc (suppression_matches_type_name): Get
	regexes by const reference.
	(suppression_matches_type_location): Get regexes by const
	reference. (function_suppression::suppresses_function): Get
	regexes by const reference; use nested if checks; simplify
	logic around symbol version checks.
	(function_suppression::suppresses_function_symbol): Get
	regexes by const reference; use nested if checks; remove
	redundant regex presence checks.
	(suppression_matches_function_name): Get regexes by const
	reference. (suppression_matches_function_sym_name): Get
	regexes by const reference.
	(suppression_matches_variable_name): Get regexes by const
	reference. (suppression_matches_variable_sym_name): Get
	regexes by const reference. (suppression_matches_type): Get
	regexes by const reference.
	(variable_suppression::suppresses_variable): Get regexes by
	const reference; use nested if checks; remove redundant
	type_name empty check.
	(variable_suppression::suppresses_variable_symbol): Get
	regexes by const reference; use nested if checks.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-suppression.cc | 167 +++++++++++++++++++----------------------
 1 file changed, 76 insertions(+), 91 deletions(-)

Comments

Matthias Maennich April 27, 2020, 12:07 p.m. UTC | #1
On Fri, Apr 24, 2020 at 10:21:26AM +0100, Giuliano Procida wrote:
>Following previous patches, various minor inconsistencies in the
>suppression regex matching code were more readily apparent.
>
>- regex_t_sptr values are fetched both by reference and by
>  value (which has a reference count mutation cost)
>- there's a mixture of code styles for testing regex presence
>    - regex = ...; if (regex && ...)
>    - if (regex = ...) if (...)
>  the latter has the advantage that the variable has a smaller scope
>  and can be given a shorter name
>- in some cases there are redundant tests (always true due to previous
>  conditional logic)
>
>This patch ensures all shared pointers to compiled regexes are fetched
>by const reference and that the code uses the nested if style of
>presence checking where possible. It simplifies some logic where
>there's redundancy.
>
>There are no behavioural changes. There may be performance
>improvements.
>
>	* src/abg-suppression.cc (suppression_matches_type_name): Get
>	regexes by const reference.
>	(suppression_matches_type_location): Get regexes by const
>	reference. (function_suppression::suppresses_function): Get
>	regexes by const reference; use nested if checks; simplify
>	logic around symbol version checks.
>	(function_suppression::suppresses_function_symbol): Get
>	regexes by const reference; use nested if checks; remove
>	redundant regex presence checks.
>	(suppression_matches_function_name): Get regexes by const
>	reference. (suppression_matches_function_sym_name): Get
>	regexes by const reference.
>	(suppression_matches_variable_name): Get regexes by const
>	reference. (suppression_matches_variable_sym_name): Get
>	regexes by const reference. (suppression_matches_type): Get
>	regexes by const reference.
>	(variable_suppression::suppresses_variable): Get regexes by
>	const reference; use nested if checks; remove redundant
>	type_name empty check.
>	(variable_suppression::suppresses_variable_symbol): Get
>	regexes by const reference; use nested if checks.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>
>---
> src/abg-suppression.cc | 167 +++++++++++++++++++----------------------
> 1 file changed, 76 insertions(+), 91 deletions(-)
>
>diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
>index 331995bf..7d411caf 100644
>--- a/src/abg-suppression.cc
>+++ b/src/abg-suppression.cc
>@@ -1053,15 +1053,14 @@ suppression_matches_type_name(const type_suppression&	s,
> 	  // If the qualified name of the considered type doesn't match
> 	  // the regular expression of the type name, then this
> 	  // suppression doesn't apply.
>-	  if (const regex_t_sptr& type_name_regex =
>-	      s.get_type_name_regex())
>+	  if (const regex_t_sptr& type_name_regex = s.get_type_name_regex())
> 	    {
> 	      if (!regex::match(type_name_regex, type_name))
> 		return false;
> 	    }
>
>-	  if (const regex_t_sptr type_name_not_regex =
>-	      s.get_type_name_not_regex())
>+	  if (const regex_t_sptr& type_name_not_regex =
>+		s.get_type_name_not_regex())
> 	    {
> 	      if (regex::match(type_name_not_regex, type_name))
> 		return false;
>@@ -1109,7 +1108,7 @@ suppression_matches_type_location(const type_suppression&	s,
>       unsigned loc_line = 0, loc_column = 0;
>       loc.expand(loc_path, loc_line, loc_column);
>
>-      if (regex_t_sptr regexp = s.get_source_location_to_keep_regex())
>+      if (const regex_t_sptr& regexp = s.get_source_location_to_keep_regex())
> 	if (regex::match(regexp, loc_path))
> 	  return false;
>
>@@ -2549,7 +2548,7 @@ function_suppression::suppresses_function(const function_decl* fn,
>     }
>
>   // check if the "name_regexp" property matches.
>-  const regex_t_sptr name_regex = get_name_regex();
>+  const regex_t_sptr& name_regex = get_name_regex();
>   if (name_regex)
>     {
>       if (!regex::match(name_regex, fname))
>@@ -2580,7 +2579,7 @@ function_suppression::suppresses_function(const function_decl* fn,
>     }
>
>   // check if the "name_not_regexp" property matches.
>-  const regex_t_sptr name_not_regex = get_name_not_regex();
>+  const regex_t_sptr& name_not_regex = get_name_not_regex();
>   if (name_not_regex)
>     {
>       if (regex::match(name_not_regex, fname))
>@@ -2624,11 +2623,9 @@ function_suppression::suppresses_function(const function_decl* fn,
>       if (fn_return_type_name != get_return_type_name())
> 	return false;
>     }
>-  else
>+  else if (const regex_t_sptr& regex = get_return_type_regex())
>     {
>-      const regex_t_sptr return_type_regex = get_return_type_regex();
>-      if (return_type_regex
>-	  && !regex::match(return_type_regex, fn_return_type_name))
>+      if (!regex::match(regex, fn_return_type_name))

Not sure, but maybe.

else if (const regex_t_sptr& regex = get_return_type_regex()
          && (!regex::match(regex, fn_return_type_name)))
          return false;

I am not too much for this, but this pattern seems to repeat throughout
the whole patch. So, for consistency sake, we could always do:
   if (regex = get_regex() && regex::match(...))

> 	return false;
>     }
>
>@@ -2664,14 +2661,15 @@ function_suppression::suppresses_function(const function_decl* fn,
>     }
>   else if (sym)
>     {
>-      const regex_t_sptr symbol_name_regex = get_symbol_name_regex();
>-      if (symbol_name_regex && !regex::match(symbol_name_regex, fn_sym_name))
>-	return false;
>+      const regex_t_sptr& symbol_name_regex = get_symbol_name_regex();
>+      const regex_t_sptr& symbol_name_not_regex = get_symbol_name_not_regex();
>
>-      const regex_t_sptr symbol_name_not_regex = get_symbol_name_not_regex();
>-      if (symbol_name_not_regex
>-	  && regex::match(symbol_name_not_regex, fn_sym_name))
>-	return false;
>+      if (symbol_name_regex)
>+	if (!regex::match(symbol_name_regex, fn_sym_name))

if (symbol_name_regex && !regex::match(symbol_name_regex, fn_sym_name))
   return false;

More examples of this further down.

Other than that:

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>+	  return false;
>+      if (symbol_name_not_regex)
>+	if (regex::match(symbol_name_not_regex, fn_sym_name))
>+	  return false;
>
>       if (get_allow_other_aliases())
> 	{
>@@ -2684,13 +2682,13 @@ function_suppression::suppresses_function(const function_decl* fn,
> 		   a && !a->is_main_symbol();
> 		   a = a->get_next_alias())
> 		{
>-		  if (symbol_name_regex
>-		      && !regex::match(symbol_name_regex, a->get_name()))
>-		    return false;
>-
>-		  if (symbol_name_not_regex
>-		      && regex::match(symbol_name_not_regex, a->get_name()))
>-		    return false;
>+		  const std::string& alias_name = a->get_name();
>+		  if (symbol_name_regex)
>+		    if (!regex::match(symbol_name_regex, alias_name))
>+		      return false;
>+		  if (symbol_name_not_regex)
>+		    if (regex::match(symbol_name_not_regex, alias_name))
>+		      return false;
> 		}
> 	    }
> 	}
>@@ -2698,17 +2696,19 @@ function_suppression::suppresses_function(const function_decl* fn,
>
>   // Check if the "symbol_version" and "symbol_version_regexp"
>   // properties match.
>-  if (sym && !get_symbol_version().empty())
>-    {
>-      if (fn_sym_version != get_symbol_version())
>-	return false;
>-    }
>-  else if (sym)
>+  if (sym)
>     {
>-      const regex_t_sptr symbol_version_regex = get_symbol_version_regex();
>-      if (symbol_version_regex
>-	  && !regex::match(symbol_version_regex, fn_sym_version))
>-	return false;
>+      if (!get_symbol_version().empty())
>+	{
>+	  if (fn_sym_version != get_symbol_version())
>+	    return false;
>+	}
>+      else
>+	{
>+	  if (const regex_t_sptr& regex = get_symbol_version_regex())
>+	    if (!regex::match(regex, fn_sym_version))
>+	      return false;
>+	}
>     }
>
>   // Check the 'parameter' property.
>@@ -2742,16 +2742,11 @@ function_suppression::suppresses_function(const function_decl* fn,
> 	      if (tn != fn_parm_type_qualified_name)
> 		return false;
> 	    }
>-	  else
>+	  else if (const regex_t_sptr& regex =
>+		     (*p)->get_parameter_type_name_regex())
> 	    {
>-	      const regex_t_sptr parm_type_name_regex =
>-		(*p)->get_parameter_type_name_regex();
>-	      if (parm_type_name_regex)
>-		{
>-		  if (!regex::match(parm_type_name_regex,
>-				    fn_parm_type_qualified_name))
>-		    return false;
>-		}
>+	      if (!regex::match(regex, fn_parm_type_qualified_name))
>+		return false;
> 	    }
> 	}
>     }
>@@ -2834,10 +2829,9 @@ function_suppression::suppresses_function_symbol(const elf_symbol* sym,
>       if (sym_name != get_symbol_name())
> 	return false;
>     }
>-  else if (get_symbol_name_regex())
>+  else if (const regex_t_sptr& regex = get_symbol_name_regex())
>     {
>-      const regex_t_sptr symbol_name_regex = get_symbol_name_regex();
>-      if (symbol_name_regex && !regex::match(symbol_name_regex, sym_name))
>+      if (!regex::match(regex, sym_name))
> 	return false;
>     }
>   else
>@@ -2849,11 +2843,9 @@ function_suppression::suppresses_function_symbol(const elf_symbol* sym,
>       if (sym_version != get_symbol_version())
> 	return false;
>     }
>-  else if (get_symbol_version_regex())
>+  else if (const regex_t_sptr& regex = get_symbol_version_regex())
>     {
>-      const regex_t_sptr symbol_version_regex = get_symbol_version_regex();
>-      if (symbol_version_regex
>-	  && !regex::match(symbol_version_regex, sym_version))
>+      if (!regex::match(regex, sym_version))
> 	return false;
>     }
>   else
>@@ -2943,12 +2935,12 @@ bool
> suppression_matches_function_name(const suppr::function_suppression& s,
> 				  const string& fn_name)
> {
>-  if (regex_t_sptr regexp = s.get_name_regex())
>+  if (const regex_t_sptr& regexp = s.get_name_regex())
>     {
>       if (!regex::match(regexp, fn_name))
> 	return false;
>     }
>-  else if (regex_t_sptr regexp = s.get_name_not_regex())
>+  else if (const regex_t_sptr& regexp = s.get_name_not_regex())
>     {
>       if (regex::match(regexp, fn_name))
> 	return false;
>@@ -2979,12 +2971,12 @@ bool
> suppression_matches_function_sym_name(const suppr::function_suppression& s,
> 				      const string& fn_linkage_name)
> {
>-  if (regex_t_sptr regexp = s.get_symbol_name_regex())
>+  if (const regex_t_sptr& regexp = s.get_symbol_name_regex())
>     {
>       if (!regex::match(regexp, fn_linkage_name))
> 	return false;
>     }
>-  else if (regex_t_sptr regexp = s.get_symbol_name_not_regex())
>+  else if (const regex_t_sptr& regexp = s.get_symbol_name_not_regex())
>     {
>       if (regex::match(regexp, fn_linkage_name))
> 	return false;
>@@ -3012,12 +3004,12 @@ bool
> suppression_matches_variable_name(const suppr::variable_suppression& s,
> 				  const string& var_name)
> {
>-  if (regex_t_sptr regexp = s.get_name_regex())
>+  if (const regex_t_sptr& regexp = s.get_name_regex())
>     {
>       if (!regex::match(regexp, var_name))
> 	return false;
>     }
>-  else if (regex_t_sptr regexp = s.get_name_not_regex())
>+  else if (const regex_t_sptr& regexp = s.get_name_not_regex())
>     {
>       if (regex::match(regexp, var_name))
> 	return false;
>@@ -3046,12 +3038,12 @@ bool
> suppression_matches_variable_sym_name(const suppr::variable_suppression& s,
> 				      const string& var_linkage_name)
> {
>-  if (regex_t_sptr regexp = s.get_symbol_name_regex())
>+  if (const regex_t_sptr& regexp = s.get_symbol_name_regex())
>     {
>       if (!regex::match(regexp, var_linkage_name))
> 	return false;
>     }
>-  else if (regex_t_sptr regexp = s.get_symbol_name_not_regex())
>+  else if (const regex_t_sptr& regexp = s.get_symbol_name_not_regex())
>     {
>       if (regex::match(regexp, var_linkage_name))
> 	return false;
>@@ -3080,7 +3072,7 @@ bool
> suppression_matches_type(const suppr::type_suppression& s,
> 			 const string& type_name)
> {
>-  if (regex_t_sptr regexp = s.get_type_name_regex())
>+  if (const regex_t_sptr& regexp = s.get_type_name_regex())
>     {
>       if (!regex::match(regexp, type_name))
> 	return false;
>@@ -3789,13 +3781,13 @@ variable_suppression::suppresses_variable(const var_decl* var,
>       // "name_regex" and "name_not_regex" properties match
>       if (get_name().empty())
> 	{
>-	  const regex_t_sptr name_regex = get_name_regex();
>-	  if (name_regex && !regex::match(name_regex, var_name))
>-	    return false;
>+	  if (const regex_t_sptr& regex = get_name_regex())
>+	    if (!regex::match(regex, var_name))
>+	      return false;
>
>-	  const regex_t_sptr name_not_regex = get_name_not_regex();
>-	  if (name_not_regex && regex::match(name_not_regex, var_name))
>-	    return false;
>+	  if (const regex_t_sptr& regex = get_name_not_regex())
>+	    if (regex::match(regex, var_name))
>+	      return false;
> 	}
>     }
>
>@@ -3809,13 +3801,13 @@ variable_suppression::suppresses_variable(const var_decl* var,
>     }
>   else
>     {
>-      const regex_t_sptr sym_name_regex = get_symbol_name_regex();
>-      if (sym_name_regex && !regex::match(sym_name_regex, var_sym_name))
>-	return false;
>+      if (const regex_t_sptr& regex = get_symbol_name_regex())
>+	if (!regex::match(regex, var_sym_name))
>+	  return false;
>
>-      const regex_t_sptr sym_name_not_regex = get_symbol_name_not_regex();
>-      if (sym_name_not_regex && regex::match(sym_name_not_regex, var_sym_name))
>-	return false;
>+      if (const regex_t_sptr& regex = get_symbol_name_not_regex())
>+	if (regex::match(regex, var_sym_name))
>+	  return false;
>     }
>
>   // Check for symbol_version and symbol_version_regexp property match
>@@ -3828,10 +3820,9 @@ variable_suppression::suppresses_variable(const var_decl* var,
>     }
>   else
>     {
>-      const regex_t_sptr symbol_version_regex = get_symbol_version_regex();
>-      if (symbol_version_regex
>-	  && !regex::match(symbol_version_regex, var_sym_version))
>-	return false;
>+      if (const regex_t_sptr& regex = get_symbol_version_regex())
>+	if (!regex::match(regex, var_sym_version))
>+	  return false;
>     }
>
>   // Check for the "type_name" and type_name_regex properties match.
>@@ -3845,12 +3836,9 @@ variable_suppression::suppresses_variable(const var_decl* var,
>     }
>   else
>     {
>-      if (get_type_name().empty())
>-	{
>-	  const regex_t_sptr type_name_regex = get_type_name_regex();
>-	  if (type_name_regex && !regex::match(type_name_regex, var_type_name))
>-	    return false;
>-	}
>+      if (const regex_t_sptr& regex = get_type_name_regex())
>+	if (!regex::match(regex, var_type_name))
>+	  return false;
>     }
>
>   return true;
>@@ -3936,10 +3924,9 @@ variable_suppression::suppresses_variable_symbol(const elf_symbol* sym,
>       if (get_symbol_name() != sym_name)
> 	return false;
>     }
>-  else if (get_symbol_name_regex())
>+  else if (const regex_t_sptr& regex = get_symbol_name_regex())
>     {
>-      const regex_t_sptr sym_name_regex = get_symbol_name_regex();
>-      if (sym_name_regex && !regex::match(sym_name_regex, sym_name))
>+      if (!regex::match(regex, sym_name))
> 	return false;
>     }
>   else
>@@ -3952,11 +3939,9 @@ variable_suppression::suppresses_variable_symbol(const elf_symbol* sym,
>       if (get_symbol_version() != sym_version)
> 	return false;
>     }
>-  else if (get_symbol_version_regex())
>+  else if (const regex_t_sptr& regex = get_symbol_version_regex())
>     {
>-      const regex_t_sptr symbol_version_regex = get_symbol_version_regex();
>-      if (symbol_version_regex
>-	  && !regex::match(symbol_version_regex, sym_version))
>+      if (!regex::match(regex, sym_version))
> 	return false;
>     }
>   else
>@@ -4257,14 +4242,14 @@ file_suppression::suppresses_file(const string& file_path)
>
>   bool has_regexp = false;
>
>-  if (regex_t_sptr regexp = get_file_name_regex())
>+  if (const regex_t_sptr& regexp = get_file_name_regex())
>     {
>       has_regexp = true;
>       if (!regex::match(regexp, fname))
> 	return false;
>     }
>
>-  if (regex_t_sptr regexp = get_file_name_not_regex())
>+  if (const regex_t_sptr& regexp = get_file_name_not_regex())
>     {
>       has_regexp = true;
>       if (regex::match(regexp, fname))
>-- 
>2.26.2.303.gf8c07b1a785-goog
>
Giuliano Procida April 27, 2020, 4:18 p.m. UTC | #2
Hi.

On Mon, 27 Apr 2020 at 13:07, Matthias Maennich <maennich@google.com> wrote:
>
> On Fri, Apr 24, 2020 at 10:21:26AM +0100, Giuliano Procida wrote:
> >Following previous patches, various minor inconsistencies in the
> >suppression regex matching code were more readily apparent.
> >
> >- regex_t_sptr values are fetched both by reference and by
> >  value (which has a reference count mutation cost)
> >- there's a mixture of code styles for testing regex presence
> >    - regex = ...; if (regex && ...)
> >    - if (regex = ...) if (...)
> >  the latter has the advantage that the variable has a smaller scope
> >  and can be given a shorter name
> >- in some cases there are redundant tests (always true due to previous
> >  conditional logic)
> >
> >This patch ensures all shared pointers to compiled regexes are fetched
> >by const reference and that the code uses the nested if style of
> >presence checking where possible. It simplifies some logic where
> >there's redundancy.
> >
> >There are no behavioural changes. There may be performance
> >improvements.
> >
> >       * src/abg-suppression.cc (suppression_matches_type_name): Get
> >       regexes by const reference.
> >       (suppression_matches_type_location): Get regexes by const
> >       reference. (function_suppression::suppresses_function): Get
> >       regexes by const reference; use nested if checks; simplify
> >       logic around symbol version checks.
> >       (function_suppression::suppresses_function_symbol): Get
> >       regexes by const reference; use nested if checks; remove
> >       redundant regex presence checks.
> >       (suppression_matches_function_name): Get regexes by const
> >       reference. (suppression_matches_function_sym_name): Get
> >       regexes by const reference.
> >       (suppression_matches_variable_name): Get regexes by const
> >       reference. (suppression_matches_variable_sym_name): Get
> >       regexes by const reference. (suppression_matches_type): Get
> >       regexes by const reference.
> >       (variable_suppression::suppresses_variable): Get regexes by
> >       const reference; use nested if checks; remove redundant
> >       type_name empty check.
> >       (variable_suppression::suppresses_variable_symbol): Get
> >       regexes by const reference; use nested if checks.
> >
> >Signed-off-by: Giuliano Procida <gprocida@google.com>
> >---
> > src/abg-suppression.cc | 167 +++++++++++++++++++----------------------
> > 1 file changed, 76 insertions(+), 91 deletions(-)
> >
> >diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
> >index 331995bf..7d411caf 100644
> >--- a/src/abg-suppression.cc
> >+++ b/src/abg-suppression.cc
> >@@ -1053,15 +1053,14 @@ suppression_matches_type_name(const type_suppression&  s,
> >         // If the qualified name of the considered type doesn't match
> >         // the regular expression of the type name, then this
> >         // suppression doesn't apply.
> >-        if (const regex_t_sptr& type_name_regex =
> >-            s.get_type_name_regex())
> >+        if (const regex_t_sptr& type_name_regex = s.get_type_name_regex())
> >           {
> >             if (!regex::match(type_name_regex, type_name))
> >               return false;
> >           }
> >
> >-        if (const regex_t_sptr type_name_not_regex =
> >-            s.get_type_name_not_regex())
> >+        if (const regex_t_sptr& type_name_not_regex =
> >+              s.get_type_name_not_regex())
> >           {
> >             if (regex::match(type_name_not_regex, type_name))
> >               return false;
> >@@ -1109,7 +1108,7 @@ suppression_matches_type_location(const type_suppression&        s,
> >       unsigned loc_line = 0, loc_column = 0;
> >       loc.expand(loc_path, loc_line, loc_column);
> >
> >-      if (regex_t_sptr regexp = s.get_source_location_to_keep_regex())
> >+      if (const regex_t_sptr& regexp = s.get_source_location_to_keep_regex())
> >       if (regex::match(regexp, loc_path))
> >         return false;
> >
> >@@ -2549,7 +2548,7 @@ function_suppression::suppresses_function(const function_decl* fn,
> >     }
> >
> >   // check if the "name_regexp" property matches.
> >-  const regex_t_sptr name_regex = get_name_regex();
> >+  const regex_t_sptr& name_regex = get_name_regex();
> >   if (name_regex)
> >     {
> >       if (!regex::match(name_regex, fname))
> >@@ -2580,7 +2579,7 @@ function_suppression::suppresses_function(const function_decl* fn,
> >     }
> >
> >   // check if the "name_not_regexp" property matches.
> >-  const regex_t_sptr name_not_regex = get_name_not_regex();
> >+  const regex_t_sptr& name_not_regex = get_name_not_regex();
> >   if (name_not_regex)
> >     {
> >       if (regex::match(name_not_regex, fname))
> >@@ -2624,11 +2623,9 @@ function_suppression::suppresses_function(const function_decl* fn,
> >       if (fn_return_type_name != get_return_type_name())
> >       return false;
> >     }
> >-  else
> >+  else if (const regex_t_sptr& regex = get_return_type_regex())
> >     {
> >-      const regex_t_sptr return_type_regex = get_return_type_regex();
> >-      if (return_type_regex
> >-        && !regex::match(return_type_regex, fn_return_type_name))
> >+      if (!regex::match(regex, fn_return_type_name))
>
> Not sure, but maybe.
>
> else if (const regex_t_sptr& regex = get_return_type_regex()
>           && (!regex::match(regex, fn_return_type_name)))
>           return false;
>
> I am not too much for this, but this pattern seems to repeat throughout
> the whole patch. So, for consistency sake, we could always do:
>    if (regex = get_regex() && regex::match(...))

That is not the syntax you are looking for...

If we want to declare regex within the conditional bracket, we can.
If we want to && within the conditional bracket, we can.
If we do both, we are initialising a regex with a boolean. I know,
b'cos I tried. :-)
Unless we use different C++17 syntax:

if (const regex_t_sptr& regex = get_regex(); regex::match(...))

> >       return false;
> >     }
> >
> >@@ -2664,14 +2661,15 @@ function_suppression::suppresses_function(const function_decl* fn,
> >     }
> >   else if (sym)
> >     {
> >-      const regex_t_sptr symbol_name_regex = get_symbol_name_regex();
> >-      if (symbol_name_regex && !regex::match(symbol_name_regex, fn_sym_name))
> >-      return false;
> >+      const regex_t_sptr& symbol_name_regex = get_symbol_name_regex();
> >+      const regex_t_sptr& symbol_name_not_regex = get_symbol_name_not_regex();
> >
> >-      const regex_t_sptr symbol_name_not_regex = get_symbol_name_not_regex();
> >-      if (symbol_name_not_regex
> >-        && regex::match(symbol_name_not_regex, fn_sym_name))
> >-      return false;
> >+      if (symbol_name_regex)
> >+      if (!regex::match(symbol_name_regex, fn_sym_name))
>
> if (symbol_name_regex && !regex::match(symbol_name_regex, fn_sym_name))
>    return false;
>
> More examples of this further down.

This is what I meant by "code uses the nested if style of presence
checking where possible". These are the only cases where the regex
isn't initialised inside the if, because we can reuse a variable. I
kept to the nested if just for consistency with all the other cases.

I'm happy to change it or discuss further.

Thanks,
Giuliano.

> Other than that:
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
>
> Cheers,
> Matthias
>
> >+        return false;
> >+      if (symbol_name_not_regex)
> >+      if (regex::match(symbol_name_not_regex, fn_sym_name))
> >+        return false;
> >
> >       if (get_allow_other_aliases())
> >       {
> >@@ -2684,13 +2682,13 @@ function_suppression::suppresses_function(const function_decl* fn,
> >                  a && !a->is_main_symbol();
> >                  a = a->get_next_alias())
> >               {
> >-                if (symbol_name_regex
> >-                    && !regex::match(symbol_name_regex, a->get_name()))
> >-                  return false;
> >-
> >-                if (symbol_name_not_regex
> >-                    && regex::match(symbol_name_not_regex, a->get_name()))
> >-                  return false;
> >+                const std::string& alias_name = a->get_name();
> >+                if (symbol_name_regex)
> >+                  if (!regex::match(symbol_name_regex, alias_name))
> >+                    return false;
> >+                if (symbol_name_not_regex)
> >+                  if (regex::match(symbol_name_not_regex, alias_name))
> >+                    return false;
> >               }
> >           }
> >       }
> >@@ -2698,17 +2696,19 @@ function_suppression::suppresses_function(const function_decl* fn,
> >
> >   // Check if the "symbol_version" and "symbol_version_regexp"
> >   // properties match.
> >-  if (sym && !get_symbol_version().empty())
> >-    {
> >-      if (fn_sym_version != get_symbol_version())
> >-      return false;
> >-    }
> >-  else if (sym)
> >+  if (sym)
> >     {
> >-      const regex_t_sptr symbol_version_regex = get_symbol_version_regex();
> >-      if (symbol_version_regex
> >-        && !regex::match(symbol_version_regex, fn_sym_version))
> >-      return false;
> >+      if (!get_symbol_version().empty())
> >+      {
> >+        if (fn_sym_version != get_symbol_version())
> >+          return false;
> >+      }
> >+      else
> >+      {
> >+        if (const regex_t_sptr& regex = get_symbol_version_regex())
> >+          if (!regex::match(regex, fn_sym_version))
> >+            return false;
> >+      }
> >     }
> >
> >   // Check the 'parameter' property.
> >@@ -2742,16 +2742,11 @@ function_suppression::suppresses_function(const function_decl* fn,
> >             if (tn != fn_parm_type_qualified_name)
> >               return false;
> >           }
> >-        else
> >+        else if (const regex_t_sptr& regex =
> >+                   (*p)->get_parameter_type_name_regex())
> >           {
> >-            const regex_t_sptr parm_type_name_regex =
> >-              (*p)->get_parameter_type_name_regex();
> >-            if (parm_type_name_regex)
> >-              {
> >-                if (!regex::match(parm_type_name_regex,
> >-                                  fn_parm_type_qualified_name))
> >-                  return false;
> >-              }
> >+            if (!regex::match(regex, fn_parm_type_qualified_name))
> >+              return false;
> >           }
> >       }
> >     }
> >@@ -2834,10 +2829,9 @@ function_suppression::suppresses_function_symbol(const elf_symbol* sym,
> >       if (sym_name != get_symbol_name())
> >       return false;
> >     }
> >-  else if (get_symbol_name_regex())
> >+  else if (const regex_t_sptr& regex = get_symbol_name_regex())
> >     {
> >-      const regex_t_sptr symbol_name_regex = get_symbol_name_regex();
> >-      if (symbol_name_regex && !regex::match(symbol_name_regex, sym_name))
> >+      if (!regex::match(regex, sym_name))
> >       return false;
> >     }
> >   else
> >@@ -2849,11 +2843,9 @@ function_suppression::suppresses_function_symbol(const elf_symbol* sym,
> >       if (sym_version != get_symbol_version())
> >       return false;
> >     }
> >-  else if (get_symbol_version_regex())
> >+  else if (const regex_t_sptr& regex = get_symbol_version_regex())
> >     {
> >-      const regex_t_sptr symbol_version_regex = get_symbol_version_regex();
> >-      if (symbol_version_regex
> >-        && !regex::match(symbol_version_regex, sym_version))
> >+      if (!regex::match(regex, sym_version))
> >       return false;
> >     }
> >   else
> >@@ -2943,12 +2935,12 @@ bool
> > suppression_matches_function_name(const suppr::function_suppression& s,
> >                                 const string& fn_name)
> > {
> >-  if (regex_t_sptr regexp = s.get_name_regex())
> >+  if (const regex_t_sptr& regexp = s.get_name_regex())
> >     {
> >       if (!regex::match(regexp, fn_name))
> >       return false;
> >     }
> >-  else if (regex_t_sptr regexp = s.get_name_not_regex())
> >+  else if (const regex_t_sptr& regexp = s.get_name_not_regex())
> >     {
> >       if (regex::match(regexp, fn_name))
> >       return false;
> >@@ -2979,12 +2971,12 @@ bool
> > suppression_matches_function_sym_name(const suppr::function_suppression& s,
> >                                     const string& fn_linkage_name)
> > {
> >-  if (regex_t_sptr regexp = s.get_symbol_name_regex())
> >+  if (const regex_t_sptr& regexp = s.get_symbol_name_regex())
> >     {
> >       if (!regex::match(regexp, fn_linkage_name))
> >       return false;
> >     }
> >-  else if (regex_t_sptr regexp = s.get_symbol_name_not_regex())
> >+  else if (const regex_t_sptr& regexp = s.get_symbol_name_not_regex())
> >     {
> >       if (regex::match(regexp, fn_linkage_name))
> >       return false;
> >@@ -3012,12 +3004,12 @@ bool
> > suppression_matches_variable_name(const suppr::variable_suppression& s,
> >                                 const string& var_name)
> > {
> >-  if (regex_t_sptr regexp = s.get_name_regex())
> >+  if (const regex_t_sptr& regexp = s.get_name_regex())
> >     {
> >       if (!regex::match(regexp, var_name))
> >       return false;
> >     }
> >-  else if (regex_t_sptr regexp = s.get_name_not_regex())
> >+  else if (const regex_t_sptr& regexp = s.get_name_not_regex())
> >     {
> >       if (regex::match(regexp, var_name))
> >       return false;
> >@@ -3046,12 +3038,12 @@ bool
> > suppression_matches_variable_sym_name(const suppr::variable_suppression& s,
> >                                     const string& var_linkage_name)
> > {
> >-  if (regex_t_sptr regexp = s.get_symbol_name_regex())
> >+  if (const regex_t_sptr& regexp = s.get_symbol_name_regex())
> >     {
> >       if (!regex::match(regexp, var_linkage_name))
> >       return false;
> >     }
> >-  else if (regex_t_sptr regexp = s.get_symbol_name_not_regex())
> >+  else if (const regex_t_sptr& regexp = s.get_symbol_name_not_regex())
> >     {
> >       if (regex::match(regexp, var_linkage_name))
> >       return false;
> >@@ -3080,7 +3072,7 @@ bool
> > suppression_matches_type(const suppr::type_suppression& s,
> >                        const string& type_name)
> > {
> >-  if (regex_t_sptr regexp = s.get_type_name_regex())
> >+  if (const regex_t_sptr& regexp = s.get_type_name_regex())
> >     {
> >       if (!regex::match(regexp, type_name))
> >       return false;
> >@@ -3789,13 +3781,13 @@ variable_suppression::suppresses_variable(const var_decl* var,
> >       // "name_regex" and "name_not_regex" properties match
> >       if (get_name().empty())
> >       {
> >-        const regex_t_sptr name_regex = get_name_regex();
> >-        if (name_regex && !regex::match(name_regex, var_name))
> >-          return false;
> >+        if (const regex_t_sptr& regex = get_name_regex())
> >+          if (!regex::match(regex, var_name))
> >+            return false;
> >
> >-        const regex_t_sptr name_not_regex = get_name_not_regex();
> >-        if (name_not_regex && regex::match(name_not_regex, var_name))
> >-          return false;
> >+        if (const regex_t_sptr& regex = get_name_not_regex())
> >+          if (regex::match(regex, var_name))
> >+            return false;
> >       }
> >     }
> >
> >@@ -3809,13 +3801,13 @@ variable_suppression::suppresses_variable(const var_decl* var,
> >     }
> >   else
> >     {
> >-      const regex_t_sptr sym_name_regex = get_symbol_name_regex();
> >-      if (sym_name_regex && !regex::match(sym_name_regex, var_sym_name))
> >-      return false;
> >+      if (const regex_t_sptr& regex = get_symbol_name_regex())
> >+      if (!regex::match(regex, var_sym_name))
> >+        return false;
> >
> >-      const regex_t_sptr sym_name_not_regex = get_symbol_name_not_regex();
> >-      if (sym_name_not_regex && regex::match(sym_name_not_regex, var_sym_name))
> >-      return false;
> >+      if (const regex_t_sptr& regex = get_symbol_name_not_regex())
> >+      if (regex::match(regex, var_sym_name))
> >+        return false;
> >     }
> >
> >   // Check for symbol_version and symbol_version_regexp property match
> >@@ -3828,10 +3820,9 @@ variable_suppression::suppresses_variable(const var_decl* var,
> >     }
> >   else
> >     {
> >-      const regex_t_sptr symbol_version_regex = get_symbol_version_regex();
> >-      if (symbol_version_regex
> >-        && !regex::match(symbol_version_regex, var_sym_version))
> >-      return false;
> >+      if (const regex_t_sptr& regex = get_symbol_version_regex())
> >+      if (!regex::match(regex, var_sym_version))
> >+        return false;
> >     }
> >
> >   // Check for the "type_name" and type_name_regex properties match.
> >@@ -3845,12 +3836,9 @@ variable_suppression::suppresses_variable(const var_decl* var,
> >     }
> >   else
> >     {
> >-      if (get_type_name().empty())
> >-      {
> >-        const regex_t_sptr type_name_regex = get_type_name_regex();
> >-        if (type_name_regex && !regex::match(type_name_regex, var_type_name))
> >-          return false;
> >-      }
> >+      if (const regex_t_sptr& regex = get_type_name_regex())
> >+      if (!regex::match(regex, var_type_name))
> >+        return false;
> >     }
> >
> >   return true;
> >@@ -3936,10 +3924,9 @@ variable_suppression::suppresses_variable_symbol(const elf_symbol* sym,
> >       if (get_symbol_name() != sym_name)
> >       return false;
> >     }
> >-  else if (get_symbol_name_regex())
> >+  else if (const regex_t_sptr& regex = get_symbol_name_regex())
> >     {
> >-      const regex_t_sptr sym_name_regex = get_symbol_name_regex();
> >-      if (sym_name_regex && !regex::match(sym_name_regex, sym_name))
> >+      if (!regex::match(regex, sym_name))
> >       return false;
> >     }
> >   else
> >@@ -3952,11 +3939,9 @@ variable_suppression::suppresses_variable_symbol(const elf_symbol* sym,
> >       if (get_symbol_version() != sym_version)
> >       return false;
> >     }
> >-  else if (get_symbol_version_regex())
> >+  else if (const regex_t_sptr& regex = get_symbol_version_regex())
> >     {
> >-      const regex_t_sptr symbol_version_regex = get_symbol_version_regex();
> >-      if (symbol_version_regex
> >-        && !regex::match(symbol_version_regex, sym_version))
> >+      if (!regex::match(regex, sym_version))
> >       return false;
> >     }
> >   else
> >@@ -4257,14 +4242,14 @@ file_suppression::suppresses_file(const string& file_path)
> >
> >   bool has_regexp = false;
> >
> >-  if (regex_t_sptr regexp = get_file_name_regex())
> >+  if (const regex_t_sptr& regexp = get_file_name_regex())
> >     {
> >       has_regexp = true;
> >       if (!regex::match(regexp, fname))
> >       return false;
> >     }
> >
> >-  if (regex_t_sptr regexp = get_file_name_not_regex())
> >+  if (const regex_t_sptr& regexp = get_file_name_not_regex())
> >     {
> >       has_regexp = true;
> >       if (regex::match(regexp, fname))
> >--
> >2.26.2.303.gf8c07b1a785-goog
> >
diff mbox series

Patch

diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
index 331995bf..7d411caf 100644
--- a/src/abg-suppression.cc
+++ b/src/abg-suppression.cc
@@ -1053,15 +1053,14 @@  suppression_matches_type_name(const type_suppression&	s,
 	  // If the qualified name of the considered type doesn't match
 	  // the regular expression of the type name, then this
 	  // suppression doesn't apply.
-	  if (const regex_t_sptr& type_name_regex =
-	      s.get_type_name_regex())
+	  if (const regex_t_sptr& type_name_regex = s.get_type_name_regex())
 	    {
 	      if (!regex::match(type_name_regex, type_name))
 		return false;
 	    }
 
-	  if (const regex_t_sptr type_name_not_regex =
-	      s.get_type_name_not_regex())
+	  if (const regex_t_sptr& type_name_not_regex =
+		s.get_type_name_not_regex())
 	    {
 	      if (regex::match(type_name_not_regex, type_name))
 		return false;
@@ -1109,7 +1108,7 @@  suppression_matches_type_location(const type_suppression&	s,
       unsigned loc_line = 0, loc_column = 0;
       loc.expand(loc_path, loc_line, loc_column);
 
-      if (regex_t_sptr regexp = s.get_source_location_to_keep_regex())
+      if (const regex_t_sptr& regexp = s.get_source_location_to_keep_regex())
 	if (regex::match(regexp, loc_path))
 	  return false;
 
@@ -2549,7 +2548,7 @@  function_suppression::suppresses_function(const function_decl* fn,
     }
 
   // check if the "name_regexp" property matches.
-  const regex_t_sptr name_regex = get_name_regex();
+  const regex_t_sptr& name_regex = get_name_regex();
   if (name_regex)
     {
       if (!regex::match(name_regex, fname))
@@ -2580,7 +2579,7 @@  function_suppression::suppresses_function(const function_decl* fn,
     }
 
   // check if the "name_not_regexp" property matches.
-  const regex_t_sptr name_not_regex = get_name_not_regex();
+  const regex_t_sptr& name_not_regex = get_name_not_regex();
   if (name_not_regex)
     {
       if (regex::match(name_not_regex, fname))
@@ -2624,11 +2623,9 @@  function_suppression::suppresses_function(const function_decl* fn,
       if (fn_return_type_name != get_return_type_name())
 	return false;
     }
-  else
+  else if (const regex_t_sptr& regex = get_return_type_regex())
     {
-      const regex_t_sptr return_type_regex = get_return_type_regex();
-      if (return_type_regex
-	  && !regex::match(return_type_regex, fn_return_type_name))
+      if (!regex::match(regex, fn_return_type_name))
 	return false;
     }
 
@@ -2664,14 +2661,15 @@  function_suppression::suppresses_function(const function_decl* fn,
     }
   else if (sym)
     {
-      const regex_t_sptr symbol_name_regex = get_symbol_name_regex();
-      if (symbol_name_regex && !regex::match(symbol_name_regex, fn_sym_name))
-	return false;
+      const regex_t_sptr& symbol_name_regex = get_symbol_name_regex();
+      const regex_t_sptr& symbol_name_not_regex = get_symbol_name_not_regex();
 
-      const regex_t_sptr symbol_name_not_regex = get_symbol_name_not_regex();
-      if (symbol_name_not_regex
-	  && regex::match(symbol_name_not_regex, fn_sym_name))
-	return false;
+      if (symbol_name_regex)
+	if (!regex::match(symbol_name_regex, fn_sym_name))
+	  return false;
+      if (symbol_name_not_regex)
+	if (regex::match(symbol_name_not_regex, fn_sym_name))
+	  return false;
 
       if (get_allow_other_aliases())
 	{
@@ -2684,13 +2682,13 @@  function_suppression::suppresses_function(const function_decl* fn,
 		   a && !a->is_main_symbol();
 		   a = a->get_next_alias())
 		{
-		  if (symbol_name_regex
-		      && !regex::match(symbol_name_regex, a->get_name()))
-		    return false;
-
-		  if (symbol_name_not_regex
-		      && regex::match(symbol_name_not_regex, a->get_name()))
-		    return false;
+		  const std::string& alias_name = a->get_name();
+		  if (symbol_name_regex)
+		    if (!regex::match(symbol_name_regex, alias_name))
+		      return false;
+		  if (symbol_name_not_regex)
+		    if (regex::match(symbol_name_not_regex, alias_name))
+		      return false;
 		}
 	    }
 	}
@@ -2698,17 +2696,19 @@  function_suppression::suppresses_function(const function_decl* fn,
 
   // Check if the "symbol_version" and "symbol_version_regexp"
   // properties match.
-  if (sym && !get_symbol_version().empty())
-    {
-      if (fn_sym_version != get_symbol_version())
-	return false;
-    }
-  else if (sym)
+  if (sym)
     {
-      const regex_t_sptr symbol_version_regex = get_symbol_version_regex();
-      if (symbol_version_regex
-	  && !regex::match(symbol_version_regex, fn_sym_version))
-	return false;
+      if (!get_symbol_version().empty())
+	{
+	  if (fn_sym_version != get_symbol_version())
+	    return false;
+	}
+      else
+	{
+	  if (const regex_t_sptr& regex = get_symbol_version_regex())
+	    if (!regex::match(regex, fn_sym_version))
+	      return false;
+	}
     }
 
   // Check the 'parameter' property.
@@ -2742,16 +2742,11 @@  function_suppression::suppresses_function(const function_decl* fn,
 	      if (tn != fn_parm_type_qualified_name)
 		return false;
 	    }
-	  else
+	  else if (const regex_t_sptr& regex =
+		     (*p)->get_parameter_type_name_regex())
 	    {
-	      const regex_t_sptr parm_type_name_regex =
-		(*p)->get_parameter_type_name_regex();
-	      if (parm_type_name_regex)
-		{
-		  if (!regex::match(parm_type_name_regex,
-				    fn_parm_type_qualified_name))
-		    return false;
-		}
+	      if (!regex::match(regex, fn_parm_type_qualified_name))
+		return false;
 	    }
 	}
     }
@@ -2834,10 +2829,9 @@  function_suppression::suppresses_function_symbol(const elf_symbol* sym,
       if (sym_name != get_symbol_name())
 	return false;
     }
-  else if (get_symbol_name_regex())
+  else if (const regex_t_sptr& regex = get_symbol_name_regex())
     {
-      const regex_t_sptr symbol_name_regex = get_symbol_name_regex();
-      if (symbol_name_regex && !regex::match(symbol_name_regex, sym_name))
+      if (!regex::match(regex, sym_name))
 	return false;
     }
   else
@@ -2849,11 +2843,9 @@  function_suppression::suppresses_function_symbol(const elf_symbol* sym,
       if (sym_version != get_symbol_version())
 	return false;
     }
-  else if (get_symbol_version_regex())
+  else if (const regex_t_sptr& regex = get_symbol_version_regex())
     {
-      const regex_t_sptr symbol_version_regex = get_symbol_version_regex();
-      if (symbol_version_regex
-	  && !regex::match(symbol_version_regex, sym_version))
+      if (!regex::match(regex, sym_version))
 	return false;
     }
   else
@@ -2943,12 +2935,12 @@  bool
 suppression_matches_function_name(const suppr::function_suppression& s,
 				  const string& fn_name)
 {
-  if (regex_t_sptr regexp = s.get_name_regex())
+  if (const regex_t_sptr& regexp = s.get_name_regex())
     {
       if (!regex::match(regexp, fn_name))
 	return false;
     }
-  else if (regex_t_sptr regexp = s.get_name_not_regex())
+  else if (const regex_t_sptr& regexp = s.get_name_not_regex())
     {
       if (regex::match(regexp, fn_name))
 	return false;
@@ -2979,12 +2971,12 @@  bool
 suppression_matches_function_sym_name(const suppr::function_suppression& s,
 				      const string& fn_linkage_name)
 {
-  if (regex_t_sptr regexp = s.get_symbol_name_regex())
+  if (const regex_t_sptr& regexp = s.get_symbol_name_regex())
     {
       if (!regex::match(regexp, fn_linkage_name))
 	return false;
     }
-  else if (regex_t_sptr regexp = s.get_symbol_name_not_regex())
+  else if (const regex_t_sptr& regexp = s.get_symbol_name_not_regex())
     {
       if (regex::match(regexp, fn_linkage_name))
 	return false;
@@ -3012,12 +3004,12 @@  bool
 suppression_matches_variable_name(const suppr::variable_suppression& s,
 				  const string& var_name)
 {
-  if (regex_t_sptr regexp = s.get_name_regex())
+  if (const regex_t_sptr& regexp = s.get_name_regex())
     {
       if (!regex::match(regexp, var_name))
 	return false;
     }
-  else if (regex_t_sptr regexp = s.get_name_not_regex())
+  else if (const regex_t_sptr& regexp = s.get_name_not_regex())
     {
       if (regex::match(regexp, var_name))
 	return false;
@@ -3046,12 +3038,12 @@  bool
 suppression_matches_variable_sym_name(const suppr::variable_suppression& s,
 				      const string& var_linkage_name)
 {
-  if (regex_t_sptr regexp = s.get_symbol_name_regex())
+  if (const regex_t_sptr& regexp = s.get_symbol_name_regex())
     {
       if (!regex::match(regexp, var_linkage_name))
 	return false;
     }
-  else if (regex_t_sptr regexp = s.get_symbol_name_not_regex())
+  else if (const regex_t_sptr& regexp = s.get_symbol_name_not_regex())
     {
       if (regex::match(regexp, var_linkage_name))
 	return false;
@@ -3080,7 +3072,7 @@  bool
 suppression_matches_type(const suppr::type_suppression& s,
 			 const string& type_name)
 {
-  if (regex_t_sptr regexp = s.get_type_name_regex())
+  if (const regex_t_sptr& regexp = s.get_type_name_regex())
     {
       if (!regex::match(regexp, type_name))
 	return false;
@@ -3789,13 +3781,13 @@  variable_suppression::suppresses_variable(const var_decl* var,
       // "name_regex" and "name_not_regex" properties match
       if (get_name().empty())
 	{
-	  const regex_t_sptr name_regex = get_name_regex();
-	  if (name_regex && !regex::match(name_regex, var_name))
-	    return false;
+	  if (const regex_t_sptr& regex = get_name_regex())
+	    if (!regex::match(regex, var_name))
+	      return false;
 
-	  const regex_t_sptr name_not_regex = get_name_not_regex();
-	  if (name_not_regex && regex::match(name_not_regex, var_name))
-	    return false;
+	  if (const regex_t_sptr& regex = get_name_not_regex())
+	    if (regex::match(regex, var_name))
+	      return false;
 	}
     }
 
@@ -3809,13 +3801,13 @@  variable_suppression::suppresses_variable(const var_decl* var,
     }
   else
     {
-      const regex_t_sptr sym_name_regex = get_symbol_name_regex();
-      if (sym_name_regex && !regex::match(sym_name_regex, var_sym_name))
-	return false;
+      if (const regex_t_sptr& regex = get_symbol_name_regex())
+	if (!regex::match(regex, var_sym_name))
+	  return false;
 
-      const regex_t_sptr sym_name_not_regex = get_symbol_name_not_regex();
-      if (sym_name_not_regex && regex::match(sym_name_not_regex, var_sym_name))
-	return false;
+      if (const regex_t_sptr& regex = get_symbol_name_not_regex())
+	if (regex::match(regex, var_sym_name))
+	  return false;
     }
 
   // Check for symbol_version and symbol_version_regexp property match
@@ -3828,10 +3820,9 @@  variable_suppression::suppresses_variable(const var_decl* var,
     }
   else
     {
-      const regex_t_sptr symbol_version_regex = get_symbol_version_regex();
-      if (symbol_version_regex
-	  && !regex::match(symbol_version_regex, var_sym_version))
-	return false;
+      if (const regex_t_sptr& regex = get_symbol_version_regex())
+	if (!regex::match(regex, var_sym_version))
+	  return false;
     }
 
   // Check for the "type_name" and type_name_regex properties match.
@@ -3845,12 +3836,9 @@  variable_suppression::suppresses_variable(const var_decl* var,
     }
   else
     {
-      if (get_type_name().empty())
-	{
-	  const regex_t_sptr type_name_regex = get_type_name_regex();
-	  if (type_name_regex && !regex::match(type_name_regex, var_type_name))
-	    return false;
-	}
+      if (const regex_t_sptr& regex = get_type_name_regex())
+	if (!regex::match(regex, var_type_name))
+	  return false;
     }
 
   return true;
@@ -3936,10 +3924,9 @@  variable_suppression::suppresses_variable_symbol(const elf_symbol* sym,
       if (get_symbol_name() != sym_name)
 	return false;
     }
-  else if (get_symbol_name_regex())
+  else if (const regex_t_sptr& regex = get_symbol_name_regex())
     {
-      const regex_t_sptr sym_name_regex = get_symbol_name_regex();
-      if (sym_name_regex && !regex::match(sym_name_regex, sym_name))
+      if (!regex::match(regex, sym_name))
 	return false;
     }
   else
@@ -3952,11 +3939,9 @@  variable_suppression::suppresses_variable_symbol(const elf_symbol* sym,
       if (get_symbol_version() != sym_version)
 	return false;
     }
-  else if (get_symbol_version_regex())
+  else if (const regex_t_sptr& regex = get_symbol_version_regex())
     {
-      const regex_t_sptr symbol_version_regex = get_symbol_version_regex();
-      if (symbol_version_regex
-	  && !regex::match(symbol_version_regex, sym_version))
+      if (!regex::match(regex, sym_version))
 	return false;
     }
   else
@@ -4257,14 +4242,14 @@  file_suppression::suppresses_file(const string& file_path)
 
   bool has_regexp = false;
 
-  if (regex_t_sptr regexp = get_file_name_regex())
+  if (const regex_t_sptr& regexp = get_file_name_regex())
     {
       has_regexp = true;
       if (!regex::match(regexp, fname))
 	return false;
     }
 
-  if (regex_t_sptr regexp = get_file_name_not_regex())
+  if (const regex_t_sptr& regexp = get_file_name_not_regex())
     {
       has_regexp = true;
       if (regex::match(regexp, fname))