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

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

[David Krauss]

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

This is my first frontend contribution. While it fixes the crash and produces an explanatory error message, the message isn't quite right. I don't understand the message generation system so I might need help. Or, it looks like there's an issue with template backtraces at the moment anyway, so there might be an interaction with another known bug.

The problem occurs when operator-> drill-down behavior is infinitely chained, for example with a template

    template< int n >
    t< n + 1 >
    t< n >::operator->()

There is no cycle to signal endlessness, and no template nesting, as drill-down is implemented as a deep expression, not tail-calls. The result is that the compiler hangs.

My solution is to pretend that there is template nesting, presuming the user will find this intuitive. There is the added benefit of the maximum chain length being configured by the template nesting limit.

Drill-down is implemented by build_x_arrow. If operator-> resolves, it calls it and uses the result type to lookup another operator->. I'd like to re-open a template context related to operator-> after generating the call. The function push_tinst_level seems to relate only to diagnostics, with no semantic effect, so it seems a good candidate.

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).

The identity of the class containing the *next* operator-> call is easy to recover, however, since it is the type of the expression from build_new_op. This introduces an off-by-one error, and gets us a class template rather than the more relevant function member. These problems shouldn't matter since this is all just for diagnostics. But perhaps the discrepancy between having a function type and a class type is interfering with message generation?

Thanks for the help and consideration!


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

* cp/typeck2.c (build_x_arrow): push_tinst_level fake context
to produce diagnostic on acyclic endless operator-> drill-down.

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



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

Index: cp/typeck2.c
===================================================================
--- cp/typeck2.c	(revision 174231)
+++ cp/typeck2.c	(working copy)
@@ -1429,24 +1429,35 @@ build_x_arrow (tree expr)
 
   if (MAYBE_CLASS_TYPE_P (type))
     {
+      struct tinst_level *actual_inst = current_instantiation ();
+      
       while ((expr = build_new_op (COMPONENT_REF, LOOKUP_NORMAL, expr,
 				   NULL_TREE, NULL_TREE,
 				   /*overloaded_p=*/NULL, 
 				   tf_warning_or_error)))
 	{
+	  tree chain_type = TREE_TYPE (expr);
+	  
 	  if (expr == error_mark_node)
 	    return error_mark_node;
 
-	  if (vec_member (TREE_TYPE (expr), types_memoized))
+	  if (vec_member (chain_type, types_memoized))
 	    {
 	      error ("circular pointer delegation detected");
 	      return error_mark_node;
 	    }
+	  
+	  /* For diagnostic purposes, pretend that the chain is nested. */
+	  if (CLASS_TYPE_P (chain_type) && CLASSTYPE_TEMPLATE_INFO (chain_type))
+	    push_tinst_level (chain_type);
 
-	  VEC_safe_push (tree, gc, types_memoized, TREE_TYPE (expr));
+	  VEC_safe_push (tree, gc, types_memoized, chain_type);
 	  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);

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

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

[Jason Merrill]

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.

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.

Jason

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

[David Krauss]

[-- 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 --]

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

[Jason Merrill]

Looks good, just need ChangeLog and testcase now.

Jason

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

[David Krauss]

On Jun 9, 2011, at 7:54 PM, Jason Merrill wrote:

> Looks good, just need ChangeLog and testcase now.
> 
> Jason

The changelog is the .clog attachment to previous.

I tried the testcase below but dejagnu seemed to hang with no compiler process running. I really don't know how to use dg, so perhaps there's an obvious error.

	- D

// { dg-do compile }

template< int n >
struct a { 
    a< n+1 >
	operator->()
	{ return a< n+1 >(); }
};

int main() {
    a<0>()->x; // { dg-error "instantiation depth exceeds maximum" }
}

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

[Jason Merrill]

Applied, thanks.

Jason