On 2/13/13, Diego Novillo <dnovi...@google.com> wrote:
> On Tue, Feb 12, 2013 at 2:47 PM, Lawrence Crowl <cr...@google.com> wrote:
>> @@ -182,24 +163,9 @@ default_emutls_var_init (tree to, tree d
>> static tree
>> get_emutls_object_type (void)
>> {
>> - tree type, type_name, field;
>> -
>> - type = emutls_object_type;
>> - if (type)
>> - return type;
>> -
>> - emutls_object_type = type = lang_hooks.types.make_type (RECORD_TYPE);
>> - type_name = NULL;
>> - field = targetm.emutls.var_fields (type, &type_name);
>> - if (!type_name)
>> - type_name = get_identifier ("__emutls_object");
>> - type_name = build_decl (UNKNOWN_LOCATION,
>> - TYPE_DECL, type_name, type);
>> - TYPE_NAME (type) = type_name;
>> - TYPE_FIELDS (type) = field;
>> - layout_type (type);
>> -
>> - return type;
>> + if (!emutls_object_type)
>> + emutls_object_type = targetm.emutls.object_type ();
>> + return emutls_object_type;
>
> Hm, this does not look like an idempotent change. Where did I
> get lost?
The responsibility for creating the type has moved into the new hook.
The old hook only had responsibility for adding fields. And changing
the name if it wasn't right. The new structure has a much clearer
division of responsibility.
> ===================================================================
>> --- gcc/coverage.c (revision 195904)
>> +++ gcc/coverage.c (working copy)
>> @@ -121,8 +121,8 @@ static const char *const ctr_names[GCOV_
>> /* Forward declarations. */
>> static void read_counts_file (void);
>> static tree build_var (tree, tree, int);
>> -static void build_fn_info_type (tree, unsigned, tree);
>> -static void build_info_type (tree, tree);
>> +static tree build_fn_info_type (unsigned, tree);
>> +static tree build_info_type (record_builder &, tree);
>
> I don't really like unnecessary forward declarations. But I guess
> it's OK, since you are replacing existing ones.
Yeah. I figure applying style changes should be a separate patch.
>> -static void
>> -build_fn_info_type (tree type, unsigned counters, tree gcov_info_type)
>> +static tree
>> +build_fn_info_type (unsigned counters, tree gcov_info_type)
>
> So, here you are changing the signature because the caller used
> to create an empty record and now it doesn't need to? We are
> going to be creating it in the constructor, right?
Correct.
--
Lawrence Crowl
