diff mbox

[9/9] ENABLE_CHECKING refactoring: C family front ends

Message ID 563628A1.2060501@gmail.com
State New
Headers show

Commit Message

Mikhail Maltsev Nov. 1, 2015, 2:58 p.m. UTC 
Hi all!

This patch was intended to be the last one in this series (but I'll send one
more cleanup patch today). It removes ENABLE_CHECKING macros in the C++ front
end (and also touches a small piece of common C family code in OpenMP).

I could convert most of "#ifdef ENABLE_CHECKING" conditions into checks
performed at run time (i.e. flag_checking), except for
GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT logic. The definition of this macro depends
on ENABLE_CHECKING (now CHECKING_P) and frankly speaking I don't know how these
checks should work (though I did not try hard to understand the details, I hope
Jason will give some comments), so I left them as-is, just got rid of
conditional compilation (i.e., used CHECKING_P).

Also, the patch renames 'check_unstripped_args' into 'verify_unstripped_args'
because I think it's more consistent with names of other internal state checking
functions.

Bootstrapped and regtested on x86_64-pc-linux-gnu with --enable-checking=yes and
--enable-checking=release. (there were some problems with languages other than C
and C++, not related to the patch, but ISTM they were fixed, so I'm currently
running a second check after rebasing).

OK for trunk?

Comments

Jeff Law Nov. 2, 2015, 11:34 p.m. UTC | #1 
On 11/01/2015 07:58 AM, Mikhail Maltsev wrote:
> Hi all!
>
> This patch was intended to be the last one in this series (but I'll send one
> more cleanup patch today). It removes ENABLE_CHECKING macros in the C++ front
> end (and also touches a small piece of common C family code in OpenMP).
>
> I could convert most of "#ifdef ENABLE_CHECKING" conditions into checks
> performed at run time (i.e. flag_checking), except for
> GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT logic. The definition of this macro depends
> on ENABLE_CHECKING (now CHECKING_P) and frankly speaking I don't know how these
> checks should work (though I did not try hard to understand the details, I hope
> Jason will give some comments), so I left them as-is, just got rid of
> conditional compilation (i.e., used CHECKING_P).
>
> Also, the patch renames 'check_unstripped_args' into 'verify_unstripped_args'
> because I think it's more consistent with names of other internal state checking
> functions.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu with --enable-checking=yes and
> --enable-checking=release. (there were some problems with languages other than C
> and C++, not related to the patch, but ISTM they were fixed, so I'm currently
> running a second check after rebasing).
>

> @@ -14279,8 +14272,9 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>         return tsubst_binary_right_fold (t, args, complain, in_decl);
>
>       default:
> -      /* We shouldn't get here, but keep going if !ENABLE_CHECKING.  */
> -      gcc_checking_assert (false);
> +      /* We shouldn't get here, but keep going if !flag_checking.  */
> +      if (!flag_checking)
> +	gcc_unreachable ();
>         return t;
>       }
>   }
I think this condition was reversed, right?

Please fix that and install on the trunk.

Thanks again!

jeff
Mikhail Maltsev Nov. 4, 2015, 2:41 p.m. UTC | #2 
On 11/03/2015 02:34 AM, Jeff Law wrote:
> 
>> @@ -14279,8 +14272,9 @@ tsubst_copy (tree t, tree args, tsubst_flags_t
>> complain, tree in_decl)
>>         return tsubst_binary_right_fold (t, args, complain, in_decl);
>>
>>       default:
>> -      /* We shouldn't get here, but keep going if !ENABLE_CHECKING.  */
>> -      gcc_checking_assert (false);
>> +      /* We shouldn't get here, but keep going if !flag_checking.  */
>> +      if (!flag_checking)
>> +    gcc_unreachable ();
>>         return t;
>>       }
>>   }
> I think this condition was reversed, right?
> 
> Please fix that and install on the trunk.
> 
> Thanks again!
> 
> jeff

Fixed and committed as r229756.
diff mbox

Patch

diff --git a/gcc/c-family/c-omp.c b/gcc/c-family/c-omp.c
index 133d079..ca64eda 100644
--- a/gcc/c-family/c-omp.c
+++ b/gcc/c-family/c-omp.c
@@ -1135,7 +1135,10 @@  c_omp_split_clauses (location_t loc, enum tree_code code,
       OMP_CLAUSE_CHAIN (clauses) = cclauses[s];
       cclauses[s] = clauses;
     }
-#ifdef ENABLE_CHECKING
+
+  if (!flag_checking)
+    return;
+
   if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_MAP)) == 0)
     gcc_assert (cclauses[C_OMP_CLAUSE_SPLIT_TARGET] == NULL_TREE);
   if ((mask & (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_NUM_TEAMS)) == 0)
@@ -1150,7 +1153,6 @@  c_omp_split_clauses (location_t loc, enum tree_code code,
     gcc_assert (cclauses[C_OMP_CLAUSE_SPLIT_FOR] == NULL_TREE);
   if (code != OMP_SIMD)
     gcc_assert (cclauses[C_OMP_CLAUSE_SPLIT_SIMD] == NULL_TREE);
-#endif
 }
 
 
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 9178188..0b7d143 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -724,8 +724,6 @@  alloc_conversion (conversion_kind kind)
   return c;
 }
 
-#ifdef ENABLE_CHECKING
-
 /* Make sure that all memory on the conversion obstack has been
    freed.  */
 
@@ -737,8 +735,6 @@  validate_conversion_obstack (void)
 		 == obstack_base (&conversion_obstack)));
 }
 
-#endif /* ENABLE_CHECKING */
-
 /* Dynamically allocate an array of N conversions.  */
 
 static conversion **
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 038c6f5..51fae5a 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3856,13 +3856,11 @@  maybe_constant_value (tree t, tree decl)
     }
 
   r = cxx_eval_outermost_constant_expr (t, true, true, decl);
-#ifdef ENABLE_CHECKING
-  gcc_assert (r == t
-	      || CONVERT_EXPR_P (t)
-	      || TREE_CODE (t) == VIEW_CONVERT_EXPR
-	      || (TREE_CONSTANT (t) && !TREE_CONSTANT (r))
-	      || !cp_tree_equal (r, t));
-#endif
+  gcc_checking_assert (r == t
+		       || CONVERT_EXPR_P (t)
+		       || TREE_CODE (t) == VIEW_CONVERT_EXPR
+		       || (TREE_CONSTANT (t) && !TREE_CONSTANT (r))
+		       || !cp_tree_equal (r, t));
   return r;
 }
 
@@ -3906,14 +3904,12 @@  fold_non_dependent_expr (tree t)
 	    }
 
 	  tree r = cxx_eval_outermost_constant_expr (t, true, true, NULL_TREE);
-#ifdef ENABLE_CHECKING
 	  /* cp_tree_equal looks through NOPs, so allow them.  */
-	  gcc_assert (r == t
-		      || CONVERT_EXPR_P (t)
-		      || TREE_CODE (t) == VIEW_CONVERT_EXPR
-		      || (TREE_CONSTANT (t) && !TREE_CONSTANT (r))
-		      || !cp_tree_equal (r, t));
-#endif
+	  gcc_checking_assert (r == t
+			       || CONVERT_EXPR_P (t)
+			       || TREE_CODE (t) == VIEW_CONVERT_EXPR
+			       || (TREE_CONSTANT (t) && !TREE_CONSTANT (r))
+			       || !cp_tree_equal (r, t));
 	  return r;
 	}
       else if (TREE_OVERFLOW_P (t))
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index adb4bae..3c54e76 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3010,7 +3010,7 @@  extern void decl_shadowed_for_var_insert (tree, tree);
    property.  */
 #define SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT(NODE, INT_VALUE) \
   NON_DEFAULT_TEMPLATE_ARGS_COUNT(NODE) = build_int_cst (NULL_TREE, INT_VALUE)
-#ifdef ENABLE_CHECKING
+#if CHECKING_P
 #define GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT(NODE) \
     int_cst_value (NON_DEFAULT_TEMPLATE_ARGS_COUNT (NODE))
 #else
@@ -5517,9 +5517,7 @@  extern tree build_cxx_call			(tree, int, tree *,
 						 tsubst_flags_t);
 extern bool is_std_init_list			(tree);
 extern bool is_list_ctor			(tree);
-#ifdef ENABLE_CHECKING
 extern void validate_conversion_obstack		(void);
-#endif /* ENABLE_CHECKING */
 extern void mark_versions_used			(tree);
 extern tree get_function_version_dispatcher	(tree);
 
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 01d4607..23f59eb 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -4923,9 +4923,8 @@  cxx_post_compilation_parsing_cleanups (void)
 
   input_location = locus_at_end_of_parsing;
 
-#ifdef ENABLE_CHECKING
-  validate_conversion_obstack ();
-#endif /* ENABLE_CHECKING */
+  if (flag_checking)
+    validate_conversion_obstack ();
 
   timevar_stop (TV_PHASE_LATE_PARSING_CLEANUPS);
 }
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index b6b9f38..182e605 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -396,20 +396,19 @@  add_substitution (tree node)
 	     get_tree_code_name (TREE_CODE (node)), (void *) node);
   node = c;
 
-#if ENABLE_CHECKING
   /* Make sure NODE isn't already a candidate.  */
-  {
-    int i;
-    tree candidate;
+  if (flag_checking)
+    {
+      int i;
+      tree candidate;
 
-    FOR_EACH_VEC_SAFE_ELT (G.substitutions, i, candidate)
-      {
-	gcc_assert (!(DECL_P (node) && node == candidate));
-	gcc_assert (!(TYPE_P (node) && TYPE_P (candidate)
+      FOR_EACH_VEC_SAFE_ELT (G.substitutions, i, candidate)
+	{
+	  gcc_assert (!(DECL_P (node) && node == candidate));
+	  gcc_assert (!(TYPE_P (node) && TYPE_P (candidate)
 		      && same_type_p (node, candidate)));
-      }
-  }
-#endif /* ENABLE_CHECKING */
+	}
+    }
 
   /* Put the decl onto the varray of substitution candidates.  */
   vec_safe_push (G.substitutions, node);
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 4de6cc2..97643b7 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1645,10 +1645,8 @@  maybe_explain_implicit_delete (tree decl)
 		    "deleted because its exception-specification does not "
 		    "match the implicit exception-specification %qX",
 		    decl, raises);
-#ifdef ENABLE_CHECKING
-	  else
+	  else if (flag_checking)
 	    gcc_unreachable ();
-#endif
 
 	  pop_scope (scope);
 	}
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 19e306d..613ed6e 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -14854,9 +14854,8 @@  cp_parser_template_argument_list (cp_parser* parser)
   parser->non_integral_constant_expression_p = saved_non_ice_p;
   parser->integral_constant_expression_p = saved_ice_p;
   parser->in_template_argument_list_p = saved_in_template_argument_list_p;
-#ifdef ENABLE_CHECKING
-  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (vec, TREE_VEC_LENGTH (vec));
-#endif
+  if (CHECKING_P)
+    SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (vec, TREE_VEC_LENGTH (vec));
   return vec;
 }
 
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index e836ec7..afb33ce 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -1134,9 +1134,8 @@  optimize_specialization_lookup_p (tree tmpl)
    gone through coerce_template_parms by now.  */
 
 static void
-check_unstripped_args (tree args ATTRIBUTE_UNUSED)
+verify_unstripped_args (tree args)
 {
-#ifdef ENABLE_CHECKING
   ++processing_template_decl;
   if (!any_dependent_template_arguments_p (args))
     {
@@ -1156,7 +1155,6 @@  check_unstripped_args (tree args ATTRIBUTE_UNUSED)
 	}
     }
   --processing_template_decl;
-#endif
 }
 
 /* Retrieve the specialization (in the sense of [temp.spec] - a
@@ -1192,7 +1190,8 @@  retrieve_specialization (tree tmpl, tree args, hashval_t hash)
 		  ? TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (tmpl))
 		  : template_class_depth (DECL_CONTEXT (tmpl))));
 
-  check_unstripped_args (args);
+  if (flag_checking)
+    verify_unstripped_args (args);
 
   if (optimize_specialization_lookup_p (tmpl))
     {
@@ -4201,10 +4200,9 @@  template_parm_to_arg (tree t)
 	  /* Turn this argument into a TYPE_ARGUMENT_PACK
 	     with a single element, which expands T.  */
 	  tree vec = make_tree_vec (1);
-#ifdef ENABLE_CHECKING
-	  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT
-	    (vec, TREE_VEC_LENGTH (vec));
-#endif
+	  if (CHECKING_P)
+	    SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (vec, TREE_VEC_LENGTH (vec));
+
 	  TREE_VEC_ELT (vec, 0) = make_pack_expansion (t);
 
 	  t = cxx_make_type (TYPE_ARGUMENT_PACK);
@@ -4221,10 +4219,9 @@  template_parm_to_arg (tree t)
 	     with a single element, which expands T.  */
 	  tree vec = make_tree_vec (1);
 	  tree type = TREE_TYPE (TEMPLATE_PARM_DECL (t));
-#ifdef ENABLE_CHECKING
-	  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT
-	    (vec, TREE_VEC_LENGTH (vec));
-#endif
+	  if (CHECKING_P)
+	    SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (vec, TREE_VEC_LENGTH (vec));
+
 	  t = convert_from_reference (t);
 	  TREE_VEC_ELT (vec, 0) = make_pack_expansion (t);
 
@@ -4265,9 +4262,8 @@  template_parms_to_args (tree parms)
       for (i = TREE_VEC_LENGTH (a) - 1; i >= 0; --i)
 	TREE_VEC_ELT (a, i) = template_parm_to_arg (TREE_VEC_ELT (a, i));
 
-#ifdef ENABLE_CHECKING
-      SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, TREE_VEC_LENGTH (a));
-#endif
+      if (CHECKING_P)
+	SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, TREE_VEC_LENGTH (a));
 
       if (length > 1)
 	TREE_VEC_ELT (args, --l) = a;
@@ -7385,10 +7381,9 @@  coerce_template_parameter_pack (tree parms,
     }
 
   SET_ARGUMENT_PACK_ARGS (argument_pack, packed_args);
-#ifdef ENABLE_CHECKING
-  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (packed_args,
-				       TREE_VEC_LENGTH (packed_args));
-#endif
+  if (CHECKING_P)
+    SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (packed_args,
+					 TREE_VEC_LENGTH (packed_args));
   return argument_pack;
 }
 
@@ -7695,11 +7690,9 @@  coerce_template_parms (tree parms,
   if (lost)
     return error_mark_node;
 
-#ifdef ENABLE_CHECKING
-  if (!NON_DEFAULT_TEMPLATE_ARGS_COUNT (new_inner_args))
+  if (CHECKING_P && !NON_DEFAULT_TEMPLATE_ARGS_COUNT (new_inner_args))
     SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (new_inner_args,
 					 TREE_VEC_LENGTH (new_inner_args));
-#endif
 
   return new_inner_args;
 }
@@ -14279,8 +14272,9 @@  tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
       return tsubst_binary_right_fold (t, args, complain, in_decl);
 
     default:
-      /* We shouldn't get here, but keep going if !ENABLE_CHECKING.  */
-      gcc_checking_assert (false);
+      /* We shouldn't get here, but keep going if !flag_checking.  */
+      if (!flag_checking)
+	gcc_unreachable ();
       return t;
     }
 }
@@ -18188,10 +18182,9 @@  type_unification_real (tree tparms,
       if (saw_undeduced++ == 1)
 	goto again;
     }
-#ifdef ENABLE_CHECKING
-  if (!NON_DEFAULT_TEMPLATE_ARGS_COUNT (targs))
+
+  if (CHECKING_P && !NON_DEFAULT_TEMPLATE_ARGS_COUNT (targs))
     SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (targs, TREE_VEC_LENGTH (targs));
-#endif
 
   return unify_success (explain_p);
 }
@@ -23239,12 +23232,10 @@  build_non_dependent_expr (tree expr)
 {
   tree inner_expr;
 
-#ifdef ENABLE_CHECKING
   /* Try to get a constant value for all non-dependent expressions in
       order to expose bugs in *_dependent_expression_p and constexpr.  */
-  if (cxx_dialect >= cxx11)
+  if (flag_checking && cxx_dialect >= cxx11)
     fold_non_dependent_expr (expr);
-#endif
 
   /* Preserve OVERLOADs; the functions must be available to resolve
      types.  */
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 11bd129..4311212 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -321,15 +321,13 @@  build_target_expr (tree decl, tree value, tsubst_flags_t complain)
   tree t;
   tree type = TREE_TYPE (decl);
 
-#ifdef ENABLE_CHECKING
-  gcc_assert (VOID_TYPE_P (TREE_TYPE (value))
-	      || TREE_TYPE (decl) == TREE_TYPE (value)
-	      /* On ARM ctors return 'this'.  */
-	      || (TYPE_PTR_P (TREE_TYPE (value))
-		  && TREE_CODE (value) == CALL_EXPR)
-	      || useless_type_conversion_p (TREE_TYPE (decl),
-					    TREE_TYPE (value)));
-#endif
+  gcc_checking_assert (VOID_TYPE_P (TREE_TYPE (value))
+		       || TREE_TYPE (decl) == TREE_TYPE (value)
+		       /* On ARM ctors return 'this'.  */
+		       || (TYPE_PTR_P (TREE_TYPE (value))
+			   && TREE_CODE (value) == CALL_EXPR)
+		       || useless_type_conversion_p (TREE_TYPE (decl),
+						     TREE_TYPE (value)));
 
   t = cxx_maybe_build_cleanup (decl, complain);
   if (t == error_mark_node)
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index e68e9df..0501e4d 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -1418,8 +1418,7 @@  comptypes (tree t1, tree t2, int strict)
 	   perform a deep check. */
 	return structural_comptypes (t1, t2, strict);
 
-#ifdef ENABLE_CHECKING
-      if (USE_CANONICAL_TYPES)
+      if (flag_checking && USE_CANONICAL_TYPES)
 	{
 	  bool result = structural_comptypes (t1, t2, strict);
 	  
@@ -1440,10 +1439,8 @@  comptypes (tree t1, tree t2, int strict)
 	  
 	  return result;
 	}
-#else
-      if (USE_CANONICAL_TYPES)
+      if (!flag_checking && USE_CANONICAL_TYPES)
 	return TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2);
-#endif
       else
 	return structural_comptypes (t1, t2, strict);
     }
diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c
index b717ea9..000f5e3 100644
--- a/gcc/cp/typeck2.c
+++ b/gcc/cp/typeck2.c
@@ -1075,11 +1075,11 @@  digest_init_r (tree type, tree init, bool nested, int flags,
 	      || TREE_CODE (type) == UNION_TYPE
 	      || TREE_CODE (type) == COMPLEX_TYPE);
 
-#ifdef ENABLE_CHECKING
   /* "If T is a class type and the initializer list has a single
      element of type cv U, where U is T or a class derived from T,
      the object is initialized from that element."  */
-  if (cxx_dialect >= cxx11
+  if (flag_checking
+      && cxx_dialect >= cxx11
       && BRACE_ENCLOSED_INITIALIZER_P (init)
       && CONSTRUCTOR_NELTS (init) == 1
       && ((CLASS_TYPE_P (type) && !CLASSTYPE_NON_AGGREGATE (type))
@@ -1090,7 +1090,6 @@  digest_init_r (tree type, tree init, bool nested, int flags,
 	/* We should have fixed this in reshape_init.  */
 	gcc_unreachable ();
     }
-#endif
 
   if (BRACE_ENCLOSED_INITIALIZER_P (init)
       && !TYPE_NON_AGGREGATE_CLASS (type))