This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
C++ PATCH: PR 13971 and PR 14086
- From: Mark Mitchell <mark at codesourcery dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Sun, 15 Feb 2004 19:14:31 -0800
- Subject: C++ PATCH: PR 13971 and PR 14086
- Reply-to: mark at codesourcery dot com
This patch fixes two more regressions in the C++ front end:
* PR c++/13971, which is a crash on a conditional-expression whose semantics
are not clearly defined by the standard. I have explained the
situation more fully in the PR, and asked the committee to open a
core issue.
* PR c++/14086, which was an ICE-on-invalid. It turned out that the
root cause was that we were checking the rule that members not have
the same name as their enclosing class in various places, but not in
a consistent way. By fixing that, I was also able to remove an n^2
algorithm (delete_duplicate_fields) in the C++ front end. (There's
no evidence that the quadradicity of that algorithm affected typical
programs, but it would have been very expensive for programs with
classes with many members.)
Tested on i686-pc-linux-gnu, applied on the mainline and on the 3.4
branch.
--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
2004-02-15 Mark Mitchell <mark@codesourcery.com>
PR c++/13971
* call.c (build_conditional_expr): Handle conversions between
class types which result in differently cv-qualified type
variants.
PR c++/14086
* class.c (delete_duplicate_fields_1): Remove.
(delete_duplicate_fields): Likewise.
(finish_struct_anon): Remove check for members with the same name
as their enclosing class.
(check_field_decls): Do not call duplicate_fields.
* decl.c (grokdeclarator): Remove check for static data members
with the same name as their enclosing class.
* name-lookup.c (push_class_level_binding): Check for members with
the same name as their enclosing class.
2004-02-15 Mark Mitchell <mark@codesourcery.com>
PR c++/13971
* g++.dg/expr/cond4.C: New test.
PR c++/14086
* g++.dg/lookup/crash2.C: New test.
Index: cp/call.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/call.c,v
retrieving revision 1.462
diff -c -5 -p -r1.462 call.c
*** cp/call.c 13 Feb 2004 20:11:33 -0000 1.462
--- cp/call.c 16 Feb 2004 02:24:38 -0000
*************** build_conditional_expr (tree arg1, tree
*** 3241,3273 ****
}
else if (conv2 && !conv2->bad_p)
{
arg2 = convert_like (conv2, arg2);
arg2 = convert_from_reference (arg2);
- if (!same_type_p (TREE_TYPE (arg2), arg3_type)
- && CLASS_TYPE_P (arg3_type))
- /* The types need to match if we're converting to a class type.
- If not, we don't care about cv-qual mismatches, since
- non-class rvalues are not cv-qualified. */
- abort ();
arg2_type = TREE_TYPE (arg2);
}
else if (conv3 && !conv3->bad_p)
{
arg3 = convert_like (conv3, arg3);
arg3 = convert_from_reference (arg3);
- if (!same_type_p (TREE_TYPE (arg3), arg2_type)
- && CLASS_TYPE_P (arg2_type))
- abort ();
arg3_type = TREE_TYPE (arg3);
}
/* Free all the conversions we allocated. */
obstack_free (&conversion_obstack, p);
if (result)
return result;
}
/* [expr.cond]
If the second and third operands are lvalues and have the same
--- 3241,3287 ----
}
else if (conv2 && !conv2->bad_p)
{
arg2 = convert_like (conv2, arg2);
arg2 = convert_from_reference (arg2);
arg2_type = TREE_TYPE (arg2);
}
else if (conv3 && !conv3->bad_p)
{
arg3 = convert_like (conv3, arg3);
arg3 = convert_from_reference (arg3);
arg3_type = TREE_TYPE (arg3);
}
/* Free all the conversions we allocated. */
obstack_free (&conversion_obstack, p);
if (result)
return result;
+
+ /* If, after the conversion, both operands have class type,
+ treat the cv-qualification of both operands as if it were the
+ union of the cv-qualification of the operands.
+
+ The standard is not clear about what to do in this
+ circumstance. For example, if the first operand has type
+ "const X" and the second operand has a user-defined
+ conversion to "volatile X", what is the type of the second
+ operand after this step? Making it be "const X" (matching
+ the first operand) seems wrong, as that discards the
+ qualification without actuall performing a copy. Leaving it
+ as "volatile X" seems wrong as that will result in the
+ conditional expression failing altogether, even though,
+ according to this step, the one operand could be converted to
+ the type of the other. */
+ if ((conv2 || conv3)
+ && CLASS_TYPE_P (arg2_type)
+ && TYPE_QUALS (arg2_type) != TYPE_QUALS (arg3_type))
+ arg2_type = arg3_type =
+ cp_build_qualified_type (arg2_type,
+ TYPE_QUALS (arg2_type)
+ | TYPE_QUALS (arg3_type));
}
/* [expr.cond]
If the second and third operands are lvalues and have the same
*************** build_conditional_expr (tree arg1, tree
*** 3347,3366 ****
performed on the second and third operands.
We need to force the lvalue-to-rvalue conversion here for class types,
so we get TARGET_EXPRs; trying to deal with a COND_EXPR of class rvalues
that isn't wrapped with a TARGET_EXPR plays havoc with exception
! regions.
!
! We use ocp_convert rather than build_user_type_conversion because the
! latter returns NULL_TREE on failure, while the former gives an error. */
arg2 = force_rvalue (arg2);
! arg2_type = TREE_TYPE (arg2);
arg3 = force_rvalue (arg3);
! arg3_type = TREE_TYPE (arg3);
if (arg2 == error_mark_node || arg3 == error_mark_node)
return error_mark_node;
/* [expr.cond]
--- 3361,3379 ----
performed on the second and third operands.
We need to force the lvalue-to-rvalue conversion here for class types,
so we get TARGET_EXPRs; trying to deal with a COND_EXPR of class rvalues
that isn't wrapped with a TARGET_EXPR plays havoc with exception
! regions. */
arg2 = force_rvalue (arg2);
! if (!CLASS_TYPE_P (arg2_type))
! arg2_type = TREE_TYPE (arg2);
arg3 = force_rvalue (arg3);
! if (!CLASS_TYPE_P (arg2_type))
! arg3_type = TREE_TYPE (arg3);
if (arg2 == error_mark_node || arg3 == error_mark_node)
return error_mark_node;
/* [expr.cond]
*************** build_conditional_expr (tree arg1, tree
*** 3443,3453 ****
throw_expr. */
/* Expand both sides into the same slot, hopefully the target of the
?: expression. We used to check for TARGET_EXPRs here, but now we
sometimes wrap them in NOP_EXPRs so the test would fail. */
! if (!lvalue_p && IS_AGGR_TYPE (TREE_TYPE (result)))
result = get_target_expr (result);
/* If this expression is an rvalue, but might be mistaken for an
lvalue, we must add a NON_LVALUE_EXPR. */
if (!lvalue_p && real_lvalue_p (result))
--- 3456,3466 ----
throw_expr. */
/* Expand both sides into the same slot, hopefully the target of the
?: expression. We used to check for TARGET_EXPRs here, but now we
sometimes wrap them in NOP_EXPRs so the test would fail. */
! if (!lvalue_p && CLASS_TYPE_P (TREE_TYPE (result)))
result = get_target_expr (result);
/* If this expression is an rvalue, but might be mistaken for an
lvalue, we must add a NON_LVALUE_EXPR. */
if (!lvalue_p && real_lvalue_p (result))
Index: cp/class.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v
retrieving revision 1.600
diff -c -5 -p -r1.600 class.c
*** cp/class.c 2 Feb 2004 16:53:01 -0000 1.600
--- cp/class.c 16 Feb 2004 02:24:39 -0000
*************** static tree get_vtable_name (tree);
*** 112,123 ****
static tree get_basefndecls (tree, tree);
static int build_primary_vtable (tree, tree);
static int build_secondary_vtable (tree);
static void finish_vtbls (tree);
static void modify_vtable_entry (tree, tree, tree, tree, tree *);
- static tree delete_duplicate_fields_1 (tree, tree);
- static void delete_duplicate_fields (tree);
static void finish_struct_bits (tree);
static int alter_access (tree, tree, tree);
static void handle_using_decl (tree, tree);
static void check_for_override (tree, tree);
static tree dfs_modify_vtables (tree, void *);
--- 112,121 ----
*************** add_method (tree type, tree method, int
*** 955,1064 ****
TREE_VEC_ELT (method_vec, slot));
}
/* Subroutines of finish_struct. */
- /* Look through the list of fields for this struct, deleting
- duplicates as we go. This must be recursive to handle
- anonymous unions.
-
- FIELD is the field which may not appear anywhere in FIELDS.
- FIELD_PTR, if non-null, is the starting point at which
- chained deletions may take place.
- The value returned is the first acceptable entry found
- in FIELDS.
-
- Note that anonymous fields which are not of UNION_TYPE are
- not duplicates, they are just anonymous fields. This happens
- when we have unnamed bitfields, for example. */
-
- static tree
- delete_duplicate_fields_1 (tree field, tree fields)
- {
- tree x;
- tree prev = 0;
- if (DECL_NAME (field) == 0)
- {
- if (! ANON_AGGR_TYPE_P (TREE_TYPE (field)))
- return fields;
-
- for (x = TYPE_FIELDS (TREE_TYPE (field)); x; x = TREE_CHAIN (x))
- fields = delete_duplicate_fields_1 (x, fields);
- return fields;
- }
- else
- {
- for (x = fields; x; prev = x, x = TREE_CHAIN (x))
- {
- if (DECL_NAME (x) == 0)
- {
- if (! ANON_AGGR_TYPE_P (TREE_TYPE (x)))
- continue;
- TYPE_FIELDS (TREE_TYPE (x))
- = delete_duplicate_fields_1 (field, TYPE_FIELDS (TREE_TYPE (x)));
- if (TYPE_FIELDS (TREE_TYPE (x)) == 0)
- {
- if (prev == 0)
- fields = TREE_CHAIN (fields);
- else
- TREE_CHAIN (prev) = TREE_CHAIN (x);
- }
- }
- else if (TREE_CODE (field) == USING_DECL)
- /* A using declaration is allowed to appear more than
- once. We'll prune these from the field list later, and
- handle_using_decl will complain about invalid multiple
- uses. */
- ;
- else if (DECL_NAME (field) == DECL_NAME (x))
- {
- if (TREE_CODE (field) == CONST_DECL
- && TREE_CODE (x) == CONST_DECL)
- cp_error_at ("duplicate enum value `%D'", x);
- else if (TREE_CODE (field) == CONST_DECL
- || TREE_CODE (x) == CONST_DECL)
- cp_error_at ("duplicate field `%D' (as enum and non-enum)",
- x);
- else if (DECL_DECLARES_TYPE_P (field)
- && DECL_DECLARES_TYPE_P (x))
- {
- if (same_type_p (TREE_TYPE (field), TREE_TYPE (x)))
- continue;
- cp_error_at ("duplicate nested type `%D'", x);
- }
- else if (DECL_DECLARES_TYPE_P (field)
- || DECL_DECLARES_TYPE_P (x))
- {
- /* Hide tag decls. */
- if ((TREE_CODE (field) == TYPE_DECL
- && DECL_ARTIFICIAL (field))
- || (TREE_CODE (x) == TYPE_DECL
- && DECL_ARTIFICIAL (x)))
- continue;
- cp_error_at ("duplicate field `%D' (as type and non-type)",
- x);
- }
- else
- cp_error_at ("duplicate member `%D'", x);
- if (prev == 0)
- fields = TREE_CHAIN (fields);
- else
- TREE_CHAIN (prev) = TREE_CHAIN (x);
- }
- }
- }
- return fields;
- }
-
- static void
- delete_duplicate_fields (tree fields)
- {
- tree x;
- for (x = fields; x && TREE_CHAIN (x); x = TREE_CHAIN (x))
- TREE_CHAIN (x) = delete_duplicate_fields_1 (x, TREE_CHAIN (x));
- }
-
/* Change the access of FDECL to ACCESS in T. Return 1 if change was
legit, otherwise return 0. */
static int
alter_access (tree t, tree fdecl, tree access)
--- 953,962 ----
*************** finish_struct_anon (tree t)
*** 2578,2591 ****
if (DECL_ARTIFICIAL (elt)
&& (!DECL_IMPLICIT_TYPEDEF_P (elt)
|| TYPE_ANONYMOUS_P (TREE_TYPE (elt))))
continue;
- if (constructor_name_p (DECL_NAME (elt), t))
- cp_pedwarn_at ("ISO C++ forbids member `%D' with same name as enclosing class",
- elt);
-
if (TREE_CODE (elt) != FIELD_DECL)
{
cp_pedwarn_at ("`%#D' invalid; an anonymous union can only have non-static data members",
elt);
continue;
--- 2476,2485 ----
*************** check_field_decls (tree t, tree *access_
*** 2957,2969 ****
{
tree *field;
tree *next;
int has_pointers;
int any_default_members;
-
- /* First, delete any duplicate fields. */
- delete_duplicate_fields (TYPE_FIELDS (t));
/* Assume there are no access declarations. */
*access_decls = NULL_TREE;
/* Assume this class has no pointer members. */
has_pointers = 0;
--- 2851,2860 ----
Index: cp/decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.1185
diff -c -5 -p -r1.1185 decl.c
*** cp/decl.c 13 Feb 2004 07:19:17 -0000 1.1185
--- cp/decl.c 16 Feb 2004 02:24:39 -0000
*************** grokdeclarator (tree declarator,
*** 8272,8288 ****
return void_type_node;
}
if (staticp)
{
- /* [class.mem] forbids static data members with the
- same name as the enclosing class. Non-static data
- members are checked in check_field_decls. */
- if (constructor_name_p (declarator, current_class_type))
- pedwarn ("ISO C++ forbids static data member `%D' with same name as enclosing class",
- declarator);
-
/* C++ allows static class members. All other work
for this is done by grokfield. */
decl = build_lang_decl (VAR_DECL, declarator, type);
TREE_STATIC (decl) = 1;
/* In class context, 'static' means public access. */
--- 8272,8281 ----
Index: cp/name-lookup.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/name-lookup.c,v
retrieving revision 1.37
diff -c -5 -p -r1.37 name-lookup.c
*** cp/name-lookup.c 8 Feb 2004 01:59:08 -0000 1.37
--- cp/name-lookup.c 16 Feb 2004 02:24:40 -0000
*************** push_class_level_binding (tree name, tre
*** 2734,2743 ****
--- 2734,2774 ----
/* Make sure that this new member does not have the same name
as a template parameter. */
if (TYPE_BEING_DEFINED (current_class_type))
check_template_shadow (x);
+ /* [class.mem]
+
+ If T is the name of a class, then each of the following shall
+ have a name different from T:
+
+ -- every static data member of class T;
+
+ -- every member of class T that is itself a type;
+
+ -- every enumerator of every member of class T that is an
+ enumerated type;
+
+ -- every member of every anonymous union that is a member of
+ class T.
+
+ (Non-static data members were also forbidden to have the same
+ name as T until TC1.) */
+ if ((TREE_CODE (x) == VAR_DECL
+ || TREE_CODE (x) == CONST_DECL
+ || (TREE_CODE (x) == TYPE_DECL
+ && !DECL_SELF_REFERENCE_P (x))
+ /* A data member of an anonymous union. */
+ || (TREE_CODE (x) == FIELD_DECL
+ && DECL_CONTEXT (x) != current_class_type))
+ && DECL_NAME (x) == constructor_name (current_class_type))
+ {
+ error ("`%D' has the same name as the class in which it is declared",
+ x);
+ POP_TIMEVAR_AND_RETURN (TV_NAME_LOOKUP, false);
+ }
+
/* If this declaration shadows a declaration from an enclosing
class, then we will need to restore IDENTIFIER_CLASS_VALUE when
we leave this class. Record the shadowed declaration here. */
binding = IDENTIFIER_BINDING (name);
if (binding && binding->value)
Index: testsuite/g++.dg/expr/cond4.C
===================================================================
RCS file: testsuite/g++.dg/expr/cond4.C
diff -N testsuite/g++.dg/expr/cond4.C
*** /dev/null 1 Jan 1970 00:00:00 -0000
--- testsuite/g++.dg/expr/cond4.C 16 Feb 2004 02:24:40 -0000
***************
*** 0 ****
--- 1,16 ----
+ // PR c++/13971
+
+ struct QChar {
+ static const QChar null;
+ };
+ struct QCharRef {
+ operator QChar() const;
+ };
+ struct QString {
+ QCharRef operator[](int i);
+ };
+
+ QChar fillParagraph(QString s, int psi) {
+ return psi ? QChar::null : s[psi];
+ }
+
Index: testsuite/g++.dg/lookup/crash2.C
===================================================================
RCS file: testsuite/g++.dg/lookup/crash2.C
diff -N testsuite/g++.dg/lookup/crash2.C
*** /dev/null 1 Jan 1970 00:00:00 -0000
--- testsuite/g++.dg/lookup/crash2.C 16 Feb 2004 02:24:40 -0000
***************
*** 0 ****
--- 1,20 ----
+ // PR c++/14086
+
+ struct ClassA
+ {
+ ClassA();
+ };
+
+ struct ClassB
+ {
+ enum Enum {ClassB}; // { dg-error "" }
+ ClassA key;
+
+ ClassB();
+ virtual ~ClassB();
+ };
+
+
+ ClassB::ClassB()
+ {
+ }