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:
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:
