On 09/20/2011 01:29 PM, Ville Voutilainen wrote:
2011-09-20 Ville Voutilainen <[email protected]>
Implement delegating constructors. Based on an
original patch by Pedro Lamarao.
Looks good, just a few minor comments:
- build2 (EQ_EXPR, boolean_type_node,
- current_in_charge_parm, integer_zero_node),
- current_vtt_parm,
- vtt);
+ build2 (EQ_EXPR, boolean_type_node,
+ current_in_charge_parm, integer_zero_node),
+ current_vtt_parm,
+ vtt);
Try to avoid reformatting lines that you don't actually change.
+ if (init == void_type_node)
+ init = NULL_TREE;
This is changing value-initialization to default-initialization, which
isn't the same thing. What happens if you just remove these two lines?
+ finish_expr_stmt (build_aggr_init (decl, init, 0, tf_warning_or_error));
Let's pass LOOKUP_NORMAL instead of 0 here.
+ LOOKUP_NONVIRTUAL|LOOKUP_DESTRUCTOR, 0,
And add LOOKUP_NORMAL| to LOOKUP_NONVIRTUAL|LOOKUP_DESTRUCTOR here.
+ if (mem_inits
+ && TYPE_P (TREE_PURPOSE (mem_inits))
+ && same_type_p (TREE_PURPOSE (mem_inits), current_class_type))
+ {
+ gcc_assert (TREE_CHAIN (mem_inits) == NULL_TREE);
+ perform_target_ctor (TREE_VALUE (mem_inits));
+ return;
+ }
+
+
Let's add a comment pointing out that this is constructor delegation.
And remove the extra blank line.
+ if (same_type_p (name, current_class_type))
+ return name;
+
basetype = TYPE_MAIN_VARIANT (name);
name = TYPE_NAME (name);
}
else if (TREE_CODE (name) == TYPE_DECL)
- basetype = TYPE_MAIN_VARIANT (TREE_TYPE (name));
+ {
+ if (same_type_p (TREE_TYPE (name), current_class_type))
+ return TREE_TYPE (name);
+ basetype = TYPE_MAIN_VARIANT (TREE_TYPE (name));
+ }
Instead of comparing to current_class_type in two places, let's compare
once in the if (basetype) block.
+ /* delegating constructor */
Generally comments in GCC should start with a capitalized word and end
with a period even if they aren't full sentences.
+ error ("seeing initializer for member %<%D%>; "
+ "previous target constructor for %T must be sole initializer",
Let's word this "mem-initializer for %qD follows constructor delegation"
+ error ("target constructor for %T must be sole initializer; "
+ "saw previous initializer for member %<%D%>",
and "constructor delegation follows mem-initializer for %qD"
We don't need to name the type again, I think.
Jason
