Re: [PATCH] [Bug c++/49118] fake template nesting for operator-> chain

[David Krauss <potswa@mac.com>]

[-- Attachment #1: Type: text/plain, Size: 2394 bytes --]

On Jun 6, 2011, at 1:28 PM, Jason Merrill wrote:

> On 06/02/2011 03:25 PM, David Krauss wrote:
>> Optimally the re-opened context would be the preceding operator->  function itself, to create the illusion of nested calls. However, the result of build_new_op may be a target_expr or a call_expr. I'm not sure of the best way to recover the function declaration from this ambiguous tree, nor whether it would a performance issue (i.e., too much work for the reward).
> 
> For a CALL_EXPR you can use get_callee_fndecl.  I wouldn't object to teaching it to handle TARGET_EXPR and AGGR_INIT_EXPR as well.

I had played around with this a bit in the meantime. It still gets stumped if operator-> is virtual. I don't know how to recover a virtual fn declaration from the tree of a virtual dispatch, but it seems to require recovery of and indirection through the vtable.

A completely different alternative is to change the overloaded_p argument of build_new_op from a bool* to a tree*. Instead of returning *whether* an overload was used, it should return *which* was used if any. This seems to be much lower impact and returns some side benefit. Perhaps the argument should be changed to a z_candidate** instead?

Patch attached. 90% of it is just stripping the "_p" from the name of the variable which is no longer a predicate. The change propagates to a few other functions, but the changes are mostly unsubstantial. I did eliminate one dead variable in cp_parser_omp_for_cond.

> As for the duplicate error message, that happens because we complain when doing push_tinst_level first in instantiate_decl and then again in build_x_arrow.  To avoid this we would need to hit each new depth first in build_x_arrow, either by pushing and then popping again before the call the build_new_op, or adding something like suspend/resume_tinst_level to pt.c.

The complaints happen inside build_new_op, not from my added push_tinst_level. Looks like I'm creating a novel condition where the return type has not been instantiated before it is used in the function call. One message results from the function body and another results from the function return type. Perhaps the multiple messages should be regarded as a separate low-priority bug. A reference return type results in three messages, all from the class template, for what it's worth.

This passes check-g++ applied to recent trunk code.

	- D
 

[-- Attachment #2: endless_arrow.clog --]
[-- Type: application/octet-stream, Size: 443 bytes --]

2011-06-09  David Krauss  <potswa@mac.com>

	PR c++/49118
	* gcc/cp/typeck2.c (build_x_arrow): Push fake template context
	to produce diagnostic on acyclic endless operator-> drill-down.
	* gcc/cp/call.c (build_new_op): Change Boolean overload status
	value to a pointer to the overload function.
	* gcc/cp/cp-tree.h: Likewise.
	* gcc/cp/typeck.c: Likewise.
	* gcc/cp/parser.c: Likewise.
	* gcc/cp/decl2.c: Likewise.
	* gcc/cp/pt.c: Likewise.

[-- Attachment #3: Type: text/plain, Size: 1 bytes --]



[-- Attachment #4: endless_arrow.patch --]
[-- Type: application/octet-stream, Size: 10005 bytes --]

Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 174231)
+++ gcc/cp/typeck.c	(working copy)
@@ -2032,7 +2032,7 @@ rationalize_conditional_expr (enum tree_
 						    ? LE_EXPR : GE_EXPR),
 						   op0, TREE_CODE (op0),
 						   op1, TREE_CODE (op1),
-						   /*overloaded_p=*/NULL,
+						   /*overload=*/NULL,
 						   complain),
                                 cp_build_unary_op (code, op0, 0, complain),
                                 cp_build_unary_op (code, op1, 0, complain),
@@ -2686,7 +2686,7 @@ build_x_indirect_ref (tree expr, ref_ope
     }
 
   rval = build_new_op (INDIRECT_REF, LOOKUP_NORMAL, expr, NULL_TREE,
-		       NULL_TREE, /*overloaded_p=*/NULL, complain);
+		       NULL_TREE, /*overload=*/NULL, complain);
   if (!rval)
     rval = cp_build_indirect_ref (expr, errorstring, complain);
 
@@ -3494,7 +3494,7 @@ convert_arguments (tree typelist, VEC(tr
 
 tree
 build_x_binary_op (enum tree_code code, tree arg1, enum tree_code arg1_code,
-		   tree arg2, enum tree_code arg2_code, bool *overloaded_p,
+		   tree arg2, enum tree_code arg2_code, tree *overload,
 		   tsubst_flags_t complain)
 {
   tree orig_arg1;
@@ -3517,7 +3517,7 @@ build_x_binary_op (enum tree_code code, 
     expr = build_m_component_ref (arg1, arg2);
   else
     expr = build_new_op (code, LOOKUP_NORMAL, arg1, arg2, NULL_TREE,
-			 overloaded_p, complain);
+			 overload, complain);
 
   /* Check for cases such as x+y<<z which users are likely to
      misinterpret.  But don't warn about obj << x + y, since that is a
@@ -3557,7 +3557,7 @@ build_x_array_ref (tree arg1, tree arg2,
     }
 
   expr = build_new_op (ARRAY_REF, LOOKUP_NORMAL, arg1, arg2, NULL_TREE,
-		       /*overloaded_p=*/NULL, complain);
+		       /*overload=*/NULL, complain);
 
   if (processing_template_decl && expr != error_mark_node)
     return build_min_non_dep (ARRAY_REF, expr, orig_arg1, orig_arg2,
@@ -4555,7 +4555,7 @@ build_x_unary_op (enum tree_code code, t
     /* Don't look for a function.  */;
   else
     exp = build_new_op (code, LOOKUP_NORMAL, xarg, NULL_TREE, NULL_TREE,
-			/*overloaded_p=*/NULL, complain);
+			/*overload=*/NULL, complain);
   if (!exp && code == ADDR_EXPR)
     {
       if (is_overloaded_fn (xarg))
@@ -5542,7 +5542,7 @@ build_x_compound_expr (tree op1, tree op
     }
 
   result = build_new_op (COMPOUND_EXPR, LOOKUP_NORMAL, op1, op2, NULL_TREE,
-			 /*overloaded_p=*/NULL, complain);
+			 /*overload=*/NULL, complain);
   if (!result)
     result = cp_build_compound_expr (op1, op2, complain);
 
@@ -6647,7 +6647,7 @@ cp_build_modify_expr (tree lhs, enum tre
 	    {
 	      result = build_new_op (MODIFY_EXPR, LOOKUP_NORMAL,
 				     lhs, rhs, make_node (NOP_EXPR),
-				     /*overloaded_p=*/NULL, 
+				     /*overload=*/NULL, 
 				     complain);
 	      if (result == NULL_TREE)
 		return error_mark_node;
@@ -6830,7 +6830,7 @@ build_x_modify_expr (tree lhs, enum tree
     {
       tree rval = build_new_op (MODIFY_EXPR, LOOKUP_NORMAL, lhs, rhs,
 				make_node (modifycode),
-				/*overloaded_p=*/NULL,
+				/*overload=*/NULL,
 				complain);
       if (rval)
 	{
Index: gcc/cp/typeck2.c
===================================================================
--- gcc/cp/typeck2.c	(revision 174789)
+++ gcc/cp/typeck2.c	(working copy)
@@ -1429,13 +1429,19 @@ build_x_arrow (tree expr)
 
   if (MAYBE_CLASS_TYPE_P (type))
     {
+      struct tinst_level *actual_inst = current_instantiation ();
+      tree fn = NULL;
+      
       while ((expr = build_new_op (COMPONENT_REF, LOOKUP_NORMAL, expr,
 				   NULL_TREE, NULL_TREE,
-				   /*overloaded_p=*/NULL, 
-				   tf_warning_or_error)))
+				   &fn, tf_warning_or_error)))
 	{
 	  if (expr == error_mark_node)
 	    return error_mark_node;
+	  
+	  if (fn && DECL_USE_TEMPLATE (fn))
+	    push_tinst_level (fn);
+	  fn = NULL;
 
 	  if (vec_member (TREE_TYPE (expr), types_memoized))
 	    {
@@ -1447,6 +1453,9 @@ build_x_arrow (tree expr)
 	  last_rval = expr;
 	}
 
+      while (current_instantiation () != actual_inst)
+	pop_tinst_level ();
+      
       if (last_rval == NULL_TREE)
 	{
 	  error ("base operand of %<->%> has non-pointer type %qT", type);
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 174231)
+++ gcc/cp/pt.c	(working copy)
@@ -12724,7 +12724,7 @@ tsubst_copy_and_build (tree t,
 	 (TREE_NO_WARNING (TREE_OPERAND (t, 1))
 	  ? ERROR_MARK
 	  : TREE_CODE (TREE_OPERAND (t, 1))),
-	 /*overloaded_p=*/NULL,
+	 /*overload=*/NULL,
 	 complain);
 
     case SCOPE_REF:
Index: gcc/cp/decl2.c
===================================================================
--- gcc/cp/decl2.c	(revision 174231)
+++ gcc/cp/decl2.c	(working copy)
@@ -361,7 +361,7 @@ grok_array_decl (tree array_expr, tree i
   if (MAYBE_CLASS_TYPE_P (type) || MAYBE_CLASS_TYPE_P (TREE_TYPE (index_exp)))
     expr = build_new_op (ARRAY_REF, LOOKUP_NORMAL,
 			 array_expr, index_exp, NULL_TREE,
-			 /*overloaded_p=*/NULL, tf_warning_or_error);
+			 /*overload=*/NULL, tf_warning_or_error);
   else
     {
       tree p1, p2, i1, i2;
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 174231)
+++ gcc/cp/parser.c	(working copy)
@@ -6579,7 +6579,7 @@ cp_parser_binary_expression (cp_parser* 
   cp_token *token;
   enum tree_code tree_type, lhs_type, rhs_type;
   enum cp_parser_prec new_prec, lookahead_prec;
-  bool overloaded_p;
+  tree overload;
 
   /* Parse the first expression.  */
   lhs = cp_parser_cast_expression (parser, /*address_p=*/false, cast_p, pidk);
@@ -6682,7 +6682,7 @@ cp_parser_binary_expression (cp_parser* 
       else if (tree_type == TRUTH_ORIF_EXPR)
 	c_inhibit_evaluation_warnings -= lhs == truthvalue_true_node;
 
-      overloaded_p = false;
+      overload = NULL;
       /* ??? Currently we pass lhs_type == ERROR_MARK and rhs_type ==
 	 ERROR_MARK for everything that is not a binary expression.
 	 This makes warn_about_parentheses miss some warnings that
@@ -6697,7 +6697,7 @@ cp_parser_binary_expression (cp_parser* 
 	lhs = build2 (tree_type, boolean_type_node, lhs, rhs);
       else
 	lhs = build_x_binary_op (tree_type, lhs, lhs_type, rhs, rhs_type,
-				 &overloaded_p, tf_warning_or_error);
+				 &overload, tf_warning_or_error);
       lhs_type = tree_type;
 
       /* If the binary operator required the use of an overloaded operator,
@@ -6706,7 +6706,7 @@ cp_parser_binary_expression (cp_parser* 
 	 otherwise permissible in an integral constant-expression if at
 	 least one of the operands is of enumeration type.  */
 
-      if (overloaded_p
+      if (overload
 	  && cp_parser_non_integral_constant_expression (parser,
 							 NIC_OVERLOADED))
 	return error_mark_node;
@@ -24221,8 +24221,6 @@ cp_parser_omp_for_cond (cp_parser *parse
 {
   tree cond = cp_parser_binary_expression (parser, false, true,
 					   PREC_NOT_OPERATOR, NULL);
-  bool overloaded_p;
-
   if (cond == error_mark_node
       || cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON))
     {
@@ -24251,7 +24249,7 @@ cp_parser_omp_for_cond (cp_parser *parse
   return build_x_binary_op (TREE_CODE (cond),
 			    TREE_OPERAND (cond, 0), ERROR_MARK,
 			    TREE_OPERAND (cond, 1), ERROR_MARK,
-			    &overloaded_p, tf_warning_or_error);
+			    /*overload=*/NULL, tf_warning_or_error);
 }
 
 /* Helper function, to parse omp for increment expression.  */
Index: gcc/cp/call.c
===================================================================
--- gcc/cp/call.c	(revision 174231)
+++ gcc/cp/call.c	(working copy)
@@ -4791,7 +4791,7 @@ avoid_sign_compare_warnings (tree orig_a
 
 static tree
 build_new_op_1 (enum tree_code code, int flags, tree arg1, tree arg2, tree arg3,
-	      bool *overloaded_p, tsubst_flags_t complain)
+		tree *overload, tsubst_flags_t complain)
 {
   tree orig_arg1 = arg1;
   tree orig_arg2 = arg2;
@@ -4958,7 +4958,7 @@ build_new_op_1 (enum tree_code code, int
 	  else
 	    code = PREDECREMENT_EXPR;
 	  result = build_new_op_1 (code, flags, arg1, NULL_TREE, NULL_TREE,
-				   overloaded_p, complain);
+				   overload, complain);
 	  break;
 
 	  /* The caller will deal with these.  */
@@ -5005,8 +5005,8 @@ build_new_op_1 (enum tree_code code, int
 	}
       else if (TREE_CODE (cand->fn) == FUNCTION_DECL)
 	{
-	  if (overloaded_p)
-	    *overloaded_p = true;
+	  if (overload)
+	    *overload = cand->fn;
 
 	  if (resolve_args (arglist, complain) == NULL)
 	    result = error_mark_node;
@@ -5165,11 +5165,11 @@ build_new_op_1 (enum tree_code code, int
 
 tree
 build_new_op (enum tree_code code, int flags, tree arg1, tree arg2, tree arg3,
-	      bool *overloaded_p, tsubst_flags_t complain)
+	      tree *overload, tsubst_flags_t complain)
 {
   tree ret;
   bool subtime = timevar_cond_start (TV_OVERLOAD);
-  ret = build_new_op_1 (code, flags, arg1, arg2, arg3, overloaded_p, complain);
+  ret = build_new_op_1 (code, flags, arg1, arg2, arg3, overload, complain);
   timevar_cond_stop (TV_OVERLOAD, subtime);
   return ret;
 }
Index: gcc/cp/cp-tree.h
===================================================================
--- gcc/cp/cp-tree.h	(revision 174231)
+++ gcc/cp/cp-tree.h	(working copy)
@@ -4703,7 +4703,7 @@ extern tree build_new_method_call		(tree
 extern tree build_special_member_call		(tree, tree, VEC(tree,gc) **,
 						 tree, int, tsubst_flags_t);
 extern tree build_new_op			(enum tree_code, int, tree, 
-						 tree, tree, bool *,
+						 tree, tree, tree *,
 						 tsubst_flags_t);
 extern tree build_op_call			(tree, VEC(tree,gc) **,
 						 tsubst_flags_t);
@@ -5597,7 +5597,7 @@ extern tree cp_build_function_call_vec		
 						 tsubst_flags_t);
 extern tree build_x_binary_op			(enum tree_code, tree,
 						 enum tree_code, tree,
-						 enum tree_code, bool *,
+						 enum tree_code, tree *,
 						 tsubst_flags_t);
 extern tree build_x_array_ref			(tree, tree, tsubst_flags_t);
 extern tree build_x_unary_op			(enum tree_code, tree,

[-- Attachment #5: Type: text/plain, Size: 2 bytes --]