On Thu, Mar 10, 2011 at 06:02:50PM -0500, Jason Merrill wrote: > On 03/10/2011 11:14 AM, Dodji Seketeli wrote: > >+ /* If we are initializing a sub-object of > >+ CURRENT_OBJECT_TYPE [for which a primary base class > >+ sub-object has already been initialized] then we must NOT > >+ initialize any sub-object from a virtual base that is a > >+ direct or indirect primary base of > >+ CURRENT_OBJECT_TYPE. */ > >+ if (current_object_type > >+ && is_virtual_base_of (TREE_TYPE (field), current_object_type) > >+ && is_primary_base_of (TREE_TYPE (field), current_object_type, > >+ /*indirect_p=*/true)) > >+ continue; > > This seems like the wrong test. If we're currently initializing a > subobject, then we don't want to initialize any of our virtual base > fields unless they are primary to the current type. We don't need > to consider the complete object type at all. > > I'm also rather nervous about using is_*_base_of tests given that a > class can have multiple indirect bases of a particular type. > Whether or not there is a virtual base T of U isn't important; what > is important is whether the current field represents a virtual base.
Do we need to redo parts of class.c anyway? From what I understand, the problematic vtbl pointers are at the end of the base types and DECL_SIZE is set to the CLASSTYPE_AS_BASE type size, thus those pointers ought to be at or beyond the containing field's size. So, can't we simply skip subfields whose bit_position is equal or larger to containing field's size? Something like (works on the testcase from this PR, otherwise untested): --- gcc/cp/init.c.jj 2011-03-11 08:06:36.000000000 +0100 +++ gcc/cp/init.c 2011-03-11 08:40:29.321401994 +0100 @@ -142,8 +142,9 @@ initialize_vtbl_ptrs (tree addr) zero-initialization does not simply mean filling the storage with zero bytes. */ -tree -build_zero_init (tree type, tree nelts, bool static_storage_p) +static tree +build_zero_init_1 (tree type, tree nelts, bool static_storage_p, + tree field_size) { tree init = NULL_TREE; @@ -188,15 +189,32 @@ build_zero_init (tree type, tree nelts, if (TREE_CODE (field) != FIELD_DECL) continue; + /* Don't add virtual bases for base classes if they are beyond + the size of the current field, that means it is present + somewhere else in the object. */ + if (field_size) + { + tree bitpos = bit_position (field); + if (TREE_CODE (bitpos) == INTEGER_CST + && !tree_int_cst_lt (bitpos, field_size)) + continue; + } + /* Note that for class types there will be FIELD_DECLs corresponding to base classes as well. Thus, iterating over TYPE_FIELDs will result in correct initialization of all of the subobjects. */ if (!static_storage_p || !zero_init_p (TREE_TYPE (field))) { - tree value = build_zero_init (TREE_TYPE (field), - /*nelts=*/NULL_TREE, - static_storage_p); + tree new_field_size + = (DECL_FIELD_IS_BASE (field) + && DECL_SIZE (field) + && TREE_CODE (DECL_SIZE (field)) == INTEGER_CST) + ? DECL_SIZE (field) : NULL_TREE; + tree value = build_zero_init_1 (TREE_TYPE (field), + /*nelts=*/NULL_TREE, + static_storage_p, + new_field_size); if (value) CONSTRUCTOR_APPEND_ELT(v, field, value); } @@ -244,9 +262,9 @@ build_zero_init (tree type, tree nelts, ce->index = build2 (RANGE_EXPR, sizetype, size_zero_node, max_index); - ce->value = build_zero_init (TREE_TYPE (type), - /*nelts=*/NULL_TREE, - static_storage_p); + ce->value = build_zero_init_1 (TREE_TYPE (type), + /*nelts=*/NULL_TREE, + static_storage_p, NULL_TREE); } /* Build a constructor to contain the initializations. */ @@ -264,6 +282,12 @@ build_zero_init (tree type, tree nelts, return init; } +tree +build_zero_init (tree type, tree nelts, bool static_storage_p) +{ + return build_zero_init_1 (type, nelts, static_storage_p, NULL_TREE); +} + /* Return a suitable initializer for value-initializing an object of type TYPE, as described in [dcl.init]. */ Jakub