Patchwork [C++] PR 54575

login
register
mail settings
Submitter Paolo Carlini
Date Sept. 14, 2012, 1:05 p.m.
Message ID <50532B88.9060100@oracle.com>
Download mbox | patch
Permalink /patch/183915/
State New
Headers show

Comments

Paolo Carlini - Sept. 14, 2012, 1:05 p.m.
Hi,

here we crash because strip_typedefs while processing _RequireInputIter 
calls make_typename_type which returns error_mark_node (# line 3281). 
Thus I'm simply checking for result == error_mark_node right after the 
switch (in the switch finish_decltype_type could also return 
error_mark_node, for example) and before handling the alignment (where 
we are crashing now). Issue seems rather straightforward.

Tested x86_64-linux.

Thanks,
Paolo.

PS: I'm also attaching a patchlet for a couple of hard errors in 
make_typename_type not protected by (complain & tf_error) spotted in 
make_typename_type.

/////////////////////////
/cp
2012-09-14  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/54575
	* tree.c (strip_typedefs): Check result for error_mark_node.
	* pt.c (canonicalize_type_argument): Check strip_typedefs return
	value for error_mark_node.

/testsuite
2012-09-14  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/54575
	* g++.dg/cpp0x/pr54575.C: New.

2012-09-14  Paolo Carlini  <paolo.carlini@oracle.com>

	* decl.c (make_typename_type): Only error out if tf_error is set
	in complain.
Index: decl.c
===================================================================
--- decl.c	(revision 191290)
+++ decl.c	(working copy)
@@ -3235,13 +3235,15 @@ make_typename_type (tree context, tree name, enum
 	name = TREE_OPERAND (fullname, 0) = DECL_NAME (name);
       else if (TREE_CODE (name) == OVERLOAD)
 	{
-	  error ("%qD is not a type", name);
+	  if (complain & tf_error)
+	    error ("%qD is not a type", name);
 	  return error_mark_node;
 	}
     }
   if (TREE_CODE (name) == TEMPLATE_DECL)
     {
-      error ("%qD used without template parameters", name);
+      if (complain & tf_error)
+	error ("%qD used without template parameters", name);
       return error_mark_node;
     }
   gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
Jason Merrill - Sept. 14, 2012, 2:36 p.m.
On 09/14/2012 09:05 AM, Paolo Carlini wrote:
> here we crash because strip_typedefs while processing _RequireInputIter
> calls make_typename_type which returns error_mark_node (# line 3281).

strip_typedefs should not return error_mark_node unless it gets 
error_mark_node as an argument; if a typedef-using type is valid, 
removing the typedefs should still produce a valid type.  I ran into a 
similar bug when I was working on the instantiation-dependent stuff, but 
I don't remember which bit fixed that one.

> PS: I'm also attaching a patchlet for a couple of hard errors in
> make_typename_type not protected by (complain & tf_error) spotted in
> make_typename_type.

This patchlet is OK.

Jason
Paolo Carlini - Sept. 14, 2012, 3:14 p.m.
Hi,

On 09/14/2012 04:36 PM, Jason Merrill wrote:
> On 09/14/2012 09:05 AM, Paolo Carlini wrote:
>> here we crash because strip_typedefs while processing _RequireInputIter
>> calls make_typename_type which returns error_mark_node (# line 3281).
>
> strip_typedefs should not return error_mark_node unless it gets 
> error_mark_node as an argument; if a typedef-using type is valid, 
> removing the typedefs should still produce a valid type.
I see, makes sense. What I'm seeing is that make_typename_type, called 
for this context (the enable_if):

  <record_type 0x7ffff694ac78 enable_if type_0 type_5 type_6 VOID
     align 8 symtab 0 alias set -1 canonical type 0x7ffff694ac78 context 
<translation_unit_decl 0x7ffff67fd170 D.1>
     full-name "struct enable_if<is_convertible<int, bool>::value>"
     no-binfo use_template=1 interface-unknown
     chain <type_decl 0x7ffff694d170 enable_if>>

wants to return error_mark_node, because here:

   if (!dependent_scope_p (context))
     /* We should only set WANT_TYPE when we're a nested typename type.
        Then we can give better diagnostics if we find a non-type. */
     t = lookup_field (context, name, 2, /*want_type=*/true);
   else
     t = NULL_TREE;

   if ((!t || TREE_CODE (t) == TREE_LIST) && dependent_type_p (context))
     return build_typename_type (context, name, fullname, tag_type);

   want_template = TREE_CODE (fullname) == TEMPLATE_ID_EXPR;

   if (!t)
     {
       if (complain & tf_error)
     error (want_template ? G_("no class template named %q#T in %q#T")
            : G_("no type named %q#T in %q#T"), name, context);
       return error_mark_node;
     }

lookup_field returns NULL_TREE and dependent_type_p (context) is false. 
It seems to me that the return value of lookup_field is right. Thus 
either dependent_type_p shouldn't be false or something is wrong in the 
logic or strip_typedefs should not call make_typename_type at all?!?

Paolo.
Paolo Carlini - Sept. 14, 2012, 4:12 p.m.
On 09/14/2012 05:14 PM, Paolo Carlini wrote:
> lookup_field returns NULL_TREE and dependent_type_p (context) is 
> false. It seems to me that the return value of lookup_field is right.
No, I don't think it makes sense for lookup_field to return NULL_TREE. 
For our testcase we should find the nested type (or we should not call 
lookup_field at all here)

Paolo.
Paolo Carlini - Sept. 14, 2012, 6:27 p.m.
Jason,

H.J. figured out that this changed when case SCOPE_REF of 
value_dependent_expression_p started always returning true, part of the 
instantiation_dependent_p work...

I'm unassigning myself, I guess you will immediately see which further 
changes are needed.

Thanks!
Paolo.

Patch

Index: testsuite/g++.dg/cpp0x/pr54575.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr54575.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr54575.C	(revision 0)
@@ -0,0 +1,27 @@ 
+// PR c++/54575
+// { dg-do compile { target c++11 } }
+
+template<typename _From, typename _To>
+struct is_convertible { static const bool value = true; };
+
+template<bool> struct enable_if       { };
+template<>     struct enable_if<true> { typedef int type; };
+
+template<typename _InIter>
+using _RequireInputIter
+= typename enable_if<is_convertible<_InIter,bool>::value>::type;
+
+template<typename _Tp>
+struct X
+{
+  template<typename _InputIterator,
+	   typename = _RequireInputIter<_InputIterator>>
+    void insert(_InputIterator) {}
+};
+
+template<typename>
+void foo()
+{
+  X<int> subdomain_indices;
+  subdomain_indices.insert(0);
+}
Index: cp/tree.c
===================================================================
--- cp/tree.c	(revision 191290)
+++ cp/tree.c	(working copy)
@@ -1210,8 +1210,10 @@  strip_typedefs (tree t)
       break;
     }
 
+  if (result == error_mark_node)
+    return error_mark_node;
   if (!result)
-      result = TYPE_MAIN_VARIANT (t);
+    result = TYPE_MAIN_VARIANT (t);
   if (TYPE_USER_ALIGN (t) != TYPE_USER_ALIGN (result)
       || TYPE_ALIGN (t) != TYPE_ALIGN (result))
     {
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 191290)
+++ cp/pt.c	(working copy)
@@ -6114,6 +6114,8 @@  canonicalize_type_argument (tree arg, tsubst_flags
     return arg;
   mv = TYPE_MAIN_VARIANT (arg);
   arg = strip_typedefs (arg);
+  if (arg == error_mark_node)
+    return error_mark_node;
   if (TYPE_ALIGN (arg) != TYPE_ALIGN (mv)
       || TYPE_ATTRIBUTES (arg) != TYPE_ATTRIBUTES (mv))
     {