Patchwork [C++] More accurate location for conditional expressions

login
register
mail settings
Submitter Paolo Carlini
Date May 21, 2013, 10:13 a.m.
Message ID <519B48CA.8020201@oracle.com>
Download mbox | patch
Permalink /patch/245255/
State New
Headers show

Comments

Paolo Carlini - May 21, 2013, 10:13 a.m.
Hi,

a little more work on locations. Yesterday, when I patched 
build_new_op_1 to propagate the incoming location to cp_build_binary_op 
I noticed that the locations for the conditional expressions in 
Wdouble-promotion.C where inaccurate, essentially always pointing to the 
end. We can improve the situation by adding a location_t parameter to 
build_conditional_expr and using it from build_x_conditional_expr. I'm 
also happy about the error message we get for cpp0x/explicit3.C: we used 
to also print spurious lines of code when printing notes about built-ins 
(eg, for line #44).

Tested x86_64-linux.

Thanks,
Paolo.

//////////////////////////
/cp
2013-05-21  Paolo Carlini  <paolo.carlini@oracle.com>

	* call.c (build_conditional_expr_1): Add location_t parameter.
	(build_conditional_expr): Likewise.
	* typeck.c (rationalize_conditional_expr, cp_build_array_ref,
	get_member_function_from_ptrfunc, build_x_conditional_expr,
	cp_build_modify_expr): Update.
	* init.c (build_new_1): Likewise.
	* cp-tree.h: Update declaration.

/testsuite
2013-05-21  Paolo Carlini  <paolo.carlini@oracle.com>

	* g++.dg/cpp0x/explicit3.C: Add column in dg-errors.
	* g++.dg/warn/Wdouble-promotion.C: Likewise.
Jason Merrill - May 21, 2013, 1:28 p.m.
On 05/21/2013 06:13 AM, Paolo Carlini wrote:
> @@ -2141,7 +2141,8 @@ rationalize_conditional_expr (enum tree_code code,
>         gcc_assert (!TREE_SIDE_EFFECTS (op0)
>   		  && !TREE_SIDE_EFFECTS (op1));
>         return
> -	build_conditional_expr (build_x_binary_op (input_location,
> +	build_conditional_expr (input_location,
> +				build_x_binary_op (input_location,
>   						   (TREE_CODE (t) == MIN_EXPR
>   						    ? LE_EXPR : GE_EXPR),
>   						   op0, TREE_CODE (op0),
> @@ -2154,7 +2155,7 @@ rationalize_conditional_expr (enum tree_code code,
>       }
>
>     return
> -    build_conditional_expr (TREE_OPERAND (t, 0),
> +    build_conditional_expr (input_location, TREE_OPERAND (t, 0),

In rationalize_conditional_expr, it seems to me that we should be using 
EXPR_LOC_OR_HERE (t) rather than input_location.  OK with that change.

Jason
Andreas Schwab - May 22, 2013, 7:26 a.m.
Paolo Carlini <paolo.carlini@oracle.com> writes:

> 	* call.c (build_conditional_expr_1): Add location_t parameter.
> 	(build_conditional_expr): Likewise.

../../gcc/objc/objc-next-runtime-abi-02.c: In function 'tree_node* build_v2_build_objc_method_call(int, tree, tree, tree, tree, bool)':
../../gcc/objc/objc-next-runtime-abi-02.c:1642:83: error: invalid conversion from 'tree' to 'location_t {aka unsigned int}' [-fpermissive]
       ret_val = build_conditional_expr (ifexp, ret_val, ftree, tf_warning_or_error);
                                                                                   ^
../../gcc/objc/objc-next-runtime-abi-02.c:1642:83: error: cannot convert 'tsubst_flags' to 'tree' for argument '4' to 'tree_node* build_conditional_expr(location_t, tree, tree, tree, tsubst_flags_t)'
make[3]: *** [objcp/objc-next-runtime-abi-02.o] Error 1

Andreas.
Paolo Carlini - May 22, 2013, 8:43 a.m.
Andreas Schwab <schwab@suse.de> ha scritto:

>Paolo Carlini <paolo.carlini@oracle.com> writes:
>
>> 	* call.c (build_conditional_expr_1): Add location_t parameter.
>> 	(build_conditional_expr): Likewise.

Argh, I'll fix it momentarily sorry. I admit I forgot to enable objc and obj-c++ when testing.

Paolo

Patch

Index: cp/call.c
===================================================================
--- cp/call.c	(revision 199137)
+++ cp/call.c	(working copy)
@@ -4353,7 +4353,7 @@  conditional_conversion (tree e1, tree e2, tsubst_f
    arguments to the conditional expression.  */
 
 static tree
-build_conditional_expr_1 (tree arg1, tree arg2, tree arg3,
+build_conditional_expr_1 (location_t loc, tree arg1, tree arg2, tree arg3,
                           tsubst_flags_t complain)
 {
   tree arg2_type;
@@ -4373,7 +4373,7 @@  static tree
   if (!arg2)
     {
       if (complain & tf_error)
-	pedwarn (input_location, OPT_Wpedantic, 
+	pedwarn (loc, OPT_Wpedantic, 
 		 "ISO C++ forbids omitting the middle term of a ?: expression");
 
       /* Make sure that lvalues remain lvalues.  See g++.oliva/ext1.C.  */
@@ -4407,8 +4407,8 @@  static tree
 	  && TREE_CODE (arg3_type) != VECTOR_TYPE)
 	{
 	  if (complain & tf_error)
-	    error ("at least one operand of a vector conditional operator "
-		   "must be a vector");
+	    error_at (loc, "at least one operand of a vector conditional "
+		      "operator must be a vector");
 	  return error_mark_node;
 	}
 
@@ -4416,7 +4416,7 @@  static tree
 	  != (TREE_CODE (arg3_type) == VECTOR_TYPE))
 	{
 	  enum stv_conv convert_flag =
-	    scalar_to_vector (input_location, VEC_COND_EXPR, arg2, arg3,
+	    scalar_to_vector (loc, VEC_COND_EXPR, arg2, arg3,
 			      complain & tf_error);
 
 	  switch (convert_flag)
@@ -4448,9 +4448,10 @@  static tree
 	  || TYPE_SIZE (arg1_type) != TYPE_SIZE (arg2_type))
 	{
 	  if (complain & tf_error)
-	    error ("incompatible vector types in conditional expression: "
-		   "%qT, %qT and %qT", TREE_TYPE (arg1), TREE_TYPE (orig_arg2),
-		   TREE_TYPE (orig_arg3));
+	    error_at (loc,
+		      "incompatible vector types in conditional expression: "
+		      "%qT, %qT and %qT", TREE_TYPE (arg1),
+		      TREE_TYPE (orig_arg2), TREE_TYPE (orig_arg3));
 	  return error_mark_node;
 	}
 
@@ -4535,15 +4536,15 @@  static tree
           if (complain & tf_error)
             {
               if (VOID_TYPE_P (arg2_type))
-                error ("second operand to the conditional operator "
-                       "is of type %<void%>, "
-                       "but the third operand is neither a throw-expression "
-                       "nor of type %<void%>");
+                error_at (EXPR_LOC_OR_LOC (arg3, loc),
+			  "second operand to the conditional operator "
+			  "is of type %<void%>, but the third operand is "
+			  "neither a throw-expression nor of type %<void%>");
               else
-                error ("third operand to the conditional operator "
-                       "is of type %<void%>, "
-                       "but the second operand is neither a throw-expression "
-                       "nor of type %<void%>");
+                error_at (EXPR_LOC_OR_LOC (arg2, loc),
+			  "third operand to the conditional operator "
+			  "is of type %<void%>, but the second operand is "
+			  "neither a throw-expression nor of type %<void%>");
             }
 	  return error_mark_node;
 	}
@@ -4583,8 +4584,8 @@  static tree
 	  || (conv3 && conv3->kind == ck_ambig))
 	{
 	  if (complain & tf_error)
-	    error ("operands to ?: have different types %qT and %qT",
-		   arg2_type, arg3_type);
+	    error_at (loc, "operands to ?: have different types %qT and %qT",
+		      arg2_type, arg3_type);
 	  result = error_mark_node;
 	}
       else if (conv2 && (!conv2->bad_p || !conv3))
@@ -4690,9 +4691,8 @@  static tree
 	{
           if (complain & tf_error)
             {
-              op_error (input_location, COND_EXPR, NOP_EXPR,
-			arg1, arg2, arg3, FALSE);
-              print_z_candidates (location_of (arg1), candidates);
+              op_error (loc, COND_EXPR, NOP_EXPR, arg1, arg2, arg3, FALSE);
+              print_z_candidates (loc, candidates);
             }
 	  return error_mark_node;
 	}
@@ -4701,9 +4701,8 @@  static tree
 	{
           if (complain & tf_error)
             {
-              op_error (input_location, COND_EXPR, NOP_EXPR,
-			arg1, arg2, arg3, FALSE);
-              print_z_candidates (location_of (arg1), candidates);
+              op_error (loc, COND_EXPR, NOP_EXPR, arg1, arg2, arg3, FALSE);
+              print_z_candidates (loc, candidates);
             }
 	  return error_mark_node;
 	}
@@ -4770,7 +4769,7 @@  static tree
 	do_warn_double_promotion (result_type, arg2_type, arg3_type,
 				  "implicit conversion from %qT to %qT to "
 				  "match other result of conditional",
-				  input_location);
+				  loc);
 
       if (TREE_CODE (arg2_type) == ENUMERAL_TYPE
 	  && TREE_CODE (arg3_type) == ENUMERAL_TYPE)
@@ -4781,19 +4780,20 @@  static tree
 	    /* Two enumerators from the same enumeration can have different
 	       types when the enumeration is still being defined.  */;
           else if (complain & tf_warning)
-            warning (OPT_Wenum_compare, 
-                     "enumeral mismatch in conditional expression: %qT vs %qT",
-                     arg2_type, arg3_type);
+            warning_at (loc, OPT_Wenum_compare, "enumeral mismatch in "
+			"conditional expression: %qT vs %qT",
+			arg2_type, arg3_type);
         }
       else if (extra_warnings
 	       && ((TREE_CODE (arg2_type) == ENUMERAL_TYPE
 		    && !same_type_p (arg3_type, type_promotes_to (arg2_type)))
 		   || (TREE_CODE (arg3_type) == ENUMERAL_TYPE
-		       && !same_type_p (arg2_type, type_promotes_to (arg3_type)))))
+		       && !same_type_p (arg2_type,
+					type_promotes_to (arg3_type)))))
         {
           if (complain & tf_warning)
-            warning (0, 
-                     "enumeral and non-enumeral type in conditional expression");
+            warning_at (loc, 0, "enumeral and non-enumeral type in "
+			"conditional expression");
         }
 
       arg2 = perform_implicit_conversion (result_type, arg2, complain);
@@ -4835,8 +4835,8 @@  static tree
   if (!result_type)
     {
       if (complain & tf_error)
-        error ("operands to ?: have different types %qT and %qT",
-               arg2_type, arg3_type);
+        error_at (loc, "operands to ?: have different types %qT and %qT",
+		  arg2_type, arg3_type);
       return error_mark_node;
     }
 
@@ -4873,12 +4873,12 @@  static tree
 /* Wrapper for above.  */
 
 tree
-build_conditional_expr (tree arg1, tree arg2, tree arg3,
+build_conditional_expr (location_t loc, tree arg1, tree arg2, tree arg3,
                         tsubst_flags_t complain)
 {
   tree ret;
   bool subtime = timevar_cond_start (TV_OVERLOAD);
-  ret = build_conditional_expr_1 (arg1, arg2, arg3, complain);
+  ret = build_conditional_expr_1 (loc, arg1, arg2, arg3, complain);
   timevar_cond_stop (TV_OVERLOAD, subtime);
   return ret;
 }
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 199137)
+++ cp/cp-tree.h	(working copy)
@@ -4969,7 +4969,7 @@  extern bool pragma_java_exceptions;
 /* in call.c */
 extern bool check_dtor_name			(tree, tree);
 
-extern tree build_conditional_expr		(tree, tree, tree, 
+extern tree build_conditional_expr		(location_t, tree, tree, tree, 
                                                  tsubst_flags_t);
 extern tree build_addr_func			(tree, tsubst_flags_t);
 extern void set_flags_from_callee		(tree);
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 199137)
+++ cp/init.c	(working copy)
@@ -2900,8 +2900,8 @@  build_new_1 (vec<tree, va_gc> **placement, tree ty
 					   NE_EXPR, alloc_node,
 					   nullptr_node,
 					   complain);
-	  rval = build_conditional_expr (ifexp, rval, alloc_node, 
-                                         complain);
+	  rval = build_conditional_expr (input_location, ifexp, rval,
+					 alloc_node, complain);
 	}
 
       /* Perform the allocation before anything else, so that ALLOC_NODE
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 199137)
+++ cp/typeck.c	(working copy)
@@ -2141,7 +2141,8 @@  rationalize_conditional_expr (enum tree_code code,
       gcc_assert (!TREE_SIDE_EFFECTS (op0)
 		  && !TREE_SIDE_EFFECTS (op1));
       return
-	build_conditional_expr (build_x_binary_op (input_location,
+	build_conditional_expr (input_location,
+				build_x_binary_op (input_location,
 						   (TREE_CODE (t) == MIN_EXPR
 						    ? LE_EXPR : GE_EXPR),
 						   op0, TREE_CODE (op0),
@@ -2154,7 +2155,7 @@  rationalize_conditional_expr (enum tree_code code,
     }
 
   return
-    build_conditional_expr (TREE_OPERAND (t, 0),
+    build_conditional_expr (input_location, TREE_OPERAND (t, 0),
 			    cp_build_unary_op (code, TREE_OPERAND (t, 1), 0,
                                                complain),
 			    cp_build_unary_op (code, TREE_OPERAND (t, 2), 0,
@@ -3024,7 +3025,7 @@  cp_build_array_ref (location_t loc, tree array, tr
 
     case COND_EXPR:
       ret = build_conditional_expr
-	      (TREE_OPERAND (array, 0),
+	       (loc, TREE_OPERAND (array, 0),
 	       cp_build_array_ref (loc, TREE_OPERAND (array, 1), idx,
 				   complain),
 	       cp_build_array_ref (loc, TREE_OPERAND (array, 2), idx,
@@ -3306,7 +3307,7 @@  get_member_function_from_ptrfunc (tree *instance_p
 		     cp_build_addr_expr (e2, complain));
 
       e2 = fold_convert (TREE_TYPE (e3), e2);
-      e1 = build_conditional_expr (e1, e2, e3, complain);
+      e1 = build_conditional_expr (input_location, e1, e2, e3, complain);
       if (e1 == error_mark_node)
 	return error_mark_node;
 
@@ -5902,7 +5903,7 @@  build_x_conditional_expr (location_t loc, tree ife
       op2 = build_non_dependent_expr (op2);
     }
 
-  expr = build_conditional_expr (ifexp, op1, op2, complain);
+  expr = build_conditional_expr (loc, ifexp, op1, op2, complain);
   if (processing_template_decl && expr != error_mark_node
       && TREE_CODE (expr) != VEC_COND_EXPR)
     {
@@ -7152,7 +7153,7 @@  cp_build_modify_expr (tree lhs, enum tree_code mod
 	  return error_mark_node;
 
 	cond = build_conditional_expr
-	  (TREE_OPERAND (lhs, 0),
+	  (input_location, TREE_OPERAND (lhs, 0),
 	   cp_build_modify_expr (TREE_OPERAND (lhs, 1),
 				 modifycode, rhs, complain),
 	   cp_build_modify_expr (TREE_OPERAND (lhs, 2),
Index: testsuite/g++.dg/cpp0x/explicit3.C
===================================================================
--- testsuite/g++.dg/cpp0x/explicit3.C	(revision 199137)
+++ testsuite/g++.dg/cpp0x/explicit3.C	(working copy)
@@ -42,10 +42,9 @@  int main()
   // These do not.
   switch (a); 			// { dg-error "" }
   bool b = a;			// { dg-error "" }
-  // { dg-message "candidate" "candidate note" { target *-*-* } 44 }
   f(a);				// { dg-error "" }
   B b2 = { a };			// { dg-error "" }
-  a + true;			// { dg-message "" }
-  b ? a : true;			// { dg-message "" }
-  a ? a : true;			// { dg-message "" }
+  a + true;			// { dg-error "5:no match" }
+  b ? a : true;			// { dg-error "5:no match" }
+  a ? a : true;			// { dg-error "5:no match" }
 }
Index: testsuite/g++.dg/warn/Wdouble-promotion.C
===================================================================
--- testsuite/g++.dg/warn/Wdouble-promotion.C	(revision 199137)
+++ testsuite/g++.dg/warn/Wdouble-promotion.C	(working copy)
@@ -29,19 +29,19 @@  usual_arithmetic_conversions(void)
   /* Values of type "float" are implicitly converted to "double" or
      "long double" due to use in arithmetic with "double" or "long
      double" operands.  */
-  local_f = f + 1.0;         /* { dg-warning "implicit" } */
-  local_f = f - d;           /* { dg-warning "implicit" } */
-  local_f = 1.0f * 1.0;      /* { dg-warning "implicit" } */
-  local_f = 1.0f / d;        /* { dg-warning "implicit" } */
+  local_f = f + 1.0;         /* { dg-warning "15:implicit" } */
+  local_f = f - d;           /* { dg-warning "15:implicit" } */
+  local_f = 1.0f * 1.0;      /* { dg-warning "18:implicit" } */
+  local_f = 1.0f / d;        /* { dg-warning "18:implicit" } */
 
-  local_cf = cf + 1.0;       /* { dg-warning "implicit" } */
-  local_cf = cf - d;         /* { dg-warning "implicit" } */
-  local_cf = cf + 1.0 * ID;  /* { dg-warning "implicit" } */
-  local_cf = cf - cd;        /* { dg-warning "implicit" } */
+  local_cf = cf + 1.0;       /* { dg-warning "17:implicit" } */
+  local_cf = cf - d;         /* { dg-warning "17:implicit" } */
+  local_cf = cf + 1.0 * ID;  /* { dg-warning "17:implicit" } */
+  local_cf = cf - cd;        /* { dg-warning "17:implicit" } */
   
-  local_f = i ? f : d;       /* { dg-warning "implicit" } */
-  i = f == d;                /* { dg-warning "implicit" } */
-  i = d != f;                /* { dg-warning "implicit" } */
+  local_f = i ? f : d;       /* { dg-warning "15:implicit" } */
+  i = f == d;                /* { dg-warning "9:implicit" } */
+  i = d != f;                /* { dg-warning "9:implicit" } */
 }
 
 void