diff mbox

[C++] PR 20420

Message ID 50355A98.4060307@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Aug. 22, 2012, 10:18 p.m. UTC
Hi again,

On 08/22/2012 05:13 PM, Jason Merrill wrote:
> On 08/22/2012 10:55 AM, Paolo Carlini wrote:
>> . thus, in short, what is happening is that, for this testcase:
>>
>> class B
>> {
>> protected:
>>    enum E { E1, E2, E3 };
>> };
>>
>> class D : private B
>> {
>> public:
>>    using B::E;
>>
>> private:
>>    enum E { };
>> };
>>
>> we parse the new declaration enum E { }; and we reach
>> supplement_binding_1 before setting the underlying type of the new
>> declaration. The old declaration is fine, would not ICE
>> dependent_type_p.
> So with your change would we still ICE if D were a template?  It seems
> like what we should be checking for is null underlying type.

In the meanwhile, I tried to actually write a patch doing away with the 
processing_template_decl check but failed.

The problem is: if, as in the below patch "3", I don't call 
dependent_type_p when the underlying type is null and the check overall 
fails, I see regressions for testcases like forw_enum7.C, because when 
templates are involved we actually want to call dependent_type_p and 
return true; if, as in the below "4", a null underlying type means that 
overall the check succeeds, then we don't error out anymore for the new 
testcases, we don't ICE but we don't have the diagnostics either.

I see the outcome of these experiments as an indication that we want the 
processing_template_decl check...

Paolo.

//////////////////////

Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c	(revision 190590)
+++ cp/name-lookup.c	(working copy)
@@ -433,20 +433,25 @@ supplement_binding_1 (cxx_binding *binding, tree d
   bool ok = true;
   tree target_bval = strip_using_decl (bval);
   tree target_decl = strip_using_decl (decl);
+  tree target_bval_type = TREE_TYPE (target_bval);
+  tree target_decl_type = TREE_TYPE (target_decl);
 
-  if (TREE_CODE (target_decl) == TYPE_DECL && DECL_ARTIFICIAL (target_decl)
+  if (TREE_CODE (target_decl) == TYPE_DECL
+      && DECL_ARTIFICIAL (target_decl)
       && target_decl != target_bval
       && (TREE_CODE (target_bval) != TYPE_DECL
 	  /* We allow pushing an enum multiple times in a class
 	     template in order to handle late matching of underlying
 	     type on an opaque-enum-declaration followed by an
 	     enum-specifier.  */
-	  || (TREE_CODE (TREE_TYPE (target_decl)) == ENUMERAL_TYPE
-	      && TREE_CODE (TREE_TYPE (target_bval)) == ENUMERAL_TYPE
-	      && (dependent_type_p (ENUM_UNDERLYING_TYPE
-				    (TREE_TYPE (target_decl)))
-		  || dependent_type_p (ENUM_UNDERLYING_TYPE
-				       (TREE_TYPE (target_bval)))))))
+	  || (TREE_CODE (target_decl_type) == ENUMERAL_TYPE
+	      && TREE_CODE (target_bval_type) == ENUMERAL_TYPE
+	      && ((! ENUM_UNDERLYING_TYPE (target_decl_type)
+		   || dependent_type_p (ENUM_UNDERLYING_TYPE
+					(target_decl_type)))
+		  || (! ENUM_UNDERLYING_TYPE (target_bval_type)
+		      || dependent_type_p (ENUM_UNDERLYING_TYPE
+					   (target_bval_type)))))))
     /* The new name is the type name.  */
     binding->type = decl;
   else if (/* TARGET_BVAL is null when push_class_level_binding moves
@@ -469,8 +474,7 @@ supplement_binding_1 (cxx_binding *binding, tree d
 	   && DECL_ARTIFICIAL (target_bval)
 	   && target_decl != target_bval
 	   && (TREE_CODE (target_decl) != TYPE_DECL
-	       || same_type_p (TREE_TYPE (target_decl),
-			       TREE_TYPE (target_bval))))
+	       || same_type_p (target_decl_type, target_bval_type)))
     {
       /* The old binding was a type name.  It was placed in
 	 VALUE field because it was thought, at the point it was
@@ -485,11 +489,11 @@ supplement_binding_1 (cxx_binding *binding, tree d
 	   && TREE_CODE (target_decl) == TYPE_DECL
 	   && DECL_NAME (target_decl) == DECL_NAME (target_bval)
 	   && binding->scope->kind != sk_class
-	   && (same_type_p (TREE_TYPE (target_decl), TREE_TYPE (target_bval))
+	   && (same_type_p (target_decl_type, target_bval_type)
 	       /* If either type involves template parameters, we must
 		  wait until instantiation.  */
-	       || uses_template_parms (TREE_TYPE (target_decl))
-	       || uses_template_parms (TREE_TYPE (target_bval))))
+	       || uses_template_parms (target_decl_type)
+	       || uses_template_parms (target_bval_type)))
     /* We have two typedef-names, both naming the same type to have
        the same name.  In general, this is OK because of:
diff mbox

Patch

Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c	(revision 190590)
+++ cp/name-lookup.c	(working copy)
@@ -433,20 +433,25 @@  supplement_binding_1 (cxx_binding *binding, tree d
   bool ok = true;
   tree target_bval = strip_using_decl (bval);
   tree target_decl = strip_using_decl (decl);
+  tree target_bval_type = TREE_TYPE (target_bval);
+  tree target_decl_type = TREE_TYPE (target_decl);
 
-  if (TREE_CODE (target_decl) == TYPE_DECL && DECL_ARTIFICIAL (target_decl)
+  if (TREE_CODE (target_decl) == TYPE_DECL
+      && DECL_ARTIFICIAL (target_decl)
       && target_decl != target_bval
       && (TREE_CODE (target_bval) != TYPE_DECL
 	  /* We allow pushing an enum multiple times in a class
 	     template in order to handle late matching of underlying
 	     type on an opaque-enum-declaration followed by an
 	     enum-specifier.  */
-	  || (TREE_CODE (TREE_TYPE (target_decl)) == ENUMERAL_TYPE
-	      && TREE_CODE (TREE_TYPE (target_bval)) == ENUMERAL_TYPE
-	      && (dependent_type_p (ENUM_UNDERLYING_TYPE
-				    (TREE_TYPE (target_decl)))
-		  || dependent_type_p (ENUM_UNDERLYING_TYPE
-				       (TREE_TYPE (target_bval)))))))
+	  || (TREE_CODE (target_decl_type) == ENUMERAL_TYPE
+	      && TREE_CODE (target_bval_type) == ENUMERAL_TYPE
+	      && ((ENUM_UNDERLYING_TYPE (target_decl_type)
+		   && dependent_type_p (ENUM_UNDERLYING_TYPE
+					(target_decl_type)))
+		  || (ENUM_UNDERLYING_TYPE (target_bval_type)
+		      && dependent_type_p (ENUM_UNDERLYING_TYPE
+					   (target_bval_type)))))))
     /* The new name is the type name.  */
     binding->type = decl;
   else if (/* TARGET_BVAL is null when push_class_level_binding moves
@@ -469,8 +474,7 @@  supplement_binding_1 (cxx_binding *binding, tree d
 	   && DECL_ARTIFICIAL (target_bval)
 	   && target_decl != target_bval
 	   && (TREE_CODE (target_decl) != TYPE_DECL
-	       || same_type_p (TREE_TYPE (target_decl),
-			       TREE_TYPE (target_bval))))
+	       || same_type_p (target_decl_type, target_bval_type)))
     {
       /* The old binding was a type name.  It was placed in
 	 VALUE field because it was thought, at the point it was
@@ -485,11 +489,11 @@  supplement_binding_1 (cxx_binding *binding, tree d
 	   && TREE_CODE (target_decl) == TYPE_DECL
 	   && DECL_NAME (target_decl) == DECL_NAME (target_bval)
 	   && binding->scope->kind != sk_class
-	   && (same_type_p (TREE_TYPE (target_decl), TREE_TYPE (target_bval))
+	   && (same_type_p (target_decl_type, target_bval_type)
 	       /* If either type involves template parameters, we must
 		  wait until instantiation.  */
-	       || uses_template_parms (TREE_TYPE (target_decl))
-	       || uses_template_parms (TREE_TYPE (target_bval))))
+	       || uses_template_parms (target_decl_type)
+	       || uses_template_parms (target_bval_type)))
     /* We have two typedef-names, both naming the same type to have
        the same name.  In general, this is OK because of: