diff mbox series

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

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

Commit Message

Giuliano Procida April 23, 2020, 3:44 p.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(-)
diff mbox series

Patch

diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
index 33bccbb6..fa382e02 100644
--- a/src/abg-suppression.cc
+++ b/src/abg-suppression.cc
@@ -1057,15 +1057,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;
@@ -1113,7 +1112,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;
 
@@ -2553,7 +2552,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))
@@ -2584,7 +2583,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))
@@ -2628,11 +2627,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;
     }
 
@@ -2668,14 +2665,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())
 	{
@@ -2688,13 +2686,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;
 		}
 	    }
 	}
@@ -2702,17 +2700,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.
@@ -2746,16 +2746,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;
 	    }
 	}
     }
@@ -2838,10 +2833,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
@@ -2853,11 +2847,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
@@ -2947,12 +2939,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;
@@ -2983,12 +2975,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;
@@ -3016,12 +3008,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;
@@ -3050,12 +3042,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;
@@ -3084,7 +3076,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;
@@ -3793,13 +3785,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;
 	}
     }
 
@@ -3813,13 +3805,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
@@ -3832,10 +3824,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.
@@ -3849,12 +3840,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;
@@ -3940,10 +3928,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
@@ -3956,11 +3943,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
@@ -4261,14 +4246,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))