diff mbox

[C++] PR 58503

Message ID 524D3E46.2070101@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Oct. 3, 2013, 9:52 a.m. UTC
Hi,

in this error recovery issue in template context, 
do_range_for_auto_deduction calls cp_parser_perform_range_for_lookup, 
which can't resolve begin/end and eventually crashes because TREE_TYPE 
(*begin) and TREE_TYPE (*end) are NULL_TREE.

It doesn't seem correct to simply early return error_mark_node, because 
the errors at issue are the permerror(s) produced by 
unqualified_fn_lookup_error. However, returning NULL_TREE, while 
adjusting do_range_for_auto_deduction to do nothing when the return type 
of begin/end isn't known, like in the case at issue, appears to work 
fine (I also checked that with -fpermissive an eventual instantiation 
produces hard errors, as it should)

Tested x86_64-linux.

Thanks,
Paolo.

////////////////////////
/cp
2013-10-03  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/58503
	* parser.c (cp_parser_perform_range_for_lookup): If either TREE_TYPE
	of *begin or *end is NULL_TREE, return NULL_TREE.
	(do_range_for_auto_deduction): If cp_parser_perform_range_for_lookup
	returns NULL_TREE, don't actually do_auto_deduction.

/testsuite
2013-10-03  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/58503
	* g++.dg/cpp0x/range-for26.C: New.

Comments

Jason Merrill Oct. 3, 2013, 1:27 p.m. UTC | #1
On 10/03/2013 05:52 AM, Paolo Carlini wrote:
> +      else if (!TREE_TYPE (*begin) || !TREE_TYPE (*end))

This should use type_dependent_expression_p.

And there should be a positive test for a dependent range that exercises 
this code.

Jason
Paolo Carlini Oct. 3, 2013, 11:42 p.m. UTC | #2
On 10/03/2013 03:27 PM, Jason Merrill wrote:
> On 10/03/2013 05:52 AM, Paolo Carlini wrote:
>> +      else if (!TREE_TYPE (*begin) || !TREE_TYPE (*end))
>
> This should use type_dependent_expression_p.
>
> And there should be a positive test for a dependent range that 
> exercises this code.

I see what you mean. But I'm not sure that positive test can really 
exist (that is, a positive test exercising the new code).

My point is that do_range_for_auto_deduction is called only by 
cp_parser_range_for and *only* when type_dependent_expression_p is 
*false* for the range_expr. Now, is it possible that in a range-based 
for-statement with such a range_expr, begin_expr and end_expr are 
nevertheless type-dependent?

At the moment I can't see how, given the various options in C++11, also 
summarized in the comment before cp_convert_range_for. Thus it seems to 
me that we are dealing with very specific forms of type-dependency that 
can occur only in invalid code. What do you think?

Thanks!
Paolo.
Jason Merrill Oct. 4, 2013, 1:41 a.m. UTC | #3
On 10/03/2013 07:42 PM, Paolo Carlini wrote:
> My point is that do_range_for_auto_deduction is called only by
> cp_parser_range_for and *only* when type_dependent_expression_p is
> *false* for the range_expr.

Aha.

> Now, is it possible that in a range-based
> for-statement with such a range_expr, begin_expr and end_expr are
> nevertheless type-dependent?

Well, the permerror is saying that with -fpermissive we'll do the lookup 
again at instantiation time, so a testcase that declares begin/end 
between the template and the instantiation ought to work?

Jason
diff mbox

Patch

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 203152)
+++ cp/parser.c	(working copy)
@@ -9960,11 +9960,15 @@  do_range_for_auto_deduction (tree decl, tree range
       range_temp = convert_from_reference (build_range_temp (range_expr));
       iter_type = (cp_parser_perform_range_for_lookup
 		   (range_temp, &begin_dummy, &end_dummy));
-      iter_decl = build_decl (input_location, VAR_DECL, NULL_TREE, iter_type);
-      iter_decl = build_x_indirect_ref (input_location, iter_decl, RO_NULL,
-					tf_warning_or_error);
-      TREE_TYPE (decl) = do_auto_deduction (TREE_TYPE (decl),
-					    iter_decl, auto_node);
+      if (iter_type)
+	{
+	  iter_decl = build_decl (input_location, VAR_DECL, NULL_TREE,
+				  iter_type);
+	  iter_decl = build_x_indirect_ref (input_location, iter_decl, RO_NULL,
+					    tf_warning_or_error);
+	  TREE_TYPE (decl) = do_auto_deduction (TREE_TYPE (decl),
+						iter_decl, auto_node);
+	}
     }
 }
 
@@ -10171,6 +10175,10 @@  cp_parser_perform_range_for_lookup (tree range, tr
 	  *begin = *end = error_mark_node;
 	  return error_mark_node;
 	}
+      else if (!TREE_TYPE (*begin) || !TREE_TYPE (*end))
+	/* Can happen, when, eg, in a template context, Koenig lookup
+	   can't resolve begin/end (c++/58503).  */
+	return NULL_TREE;
       else
 	{
 	  tree iter_type = cv_unqualified (TREE_TYPE (*begin));
Index: testsuite/g++.dg/cpp0x/range-for26.C
===================================================================
--- testsuite/g++.dg/cpp0x/range-for26.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/range-for26.C	(working copy)
@@ -0,0 +1,7 @@ 
+// PR c++/58503
+// { dg-require-effective-target c++11 }
+
+template<int> void foo()
+{
+  for (auto i : 0) {}  // { dg-error "there are no arguments" }
+}