This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH, vtv update] Change fixed size array to a vector; fix diagnostic messages.
- From: Caroline Tice <cmtice at google dot com>
- To: Diego Novillo <dnovillo at google dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Benjamin Kosnik <bkoz at redhat dot com>, "Joseph S. Myers" <joseph at codesourcery dot com>, Jason Merrill <jason at redhat dot com>
- Date: Wed, 28 Aug 2013 10:39:07 -0700
- Subject: Re: [PATCH, vtv update] Change fixed size array to a vector; fix diagnostic messages.
- Authentication-results: sourceware.org; auth=none
- References: <CABtf2+RehdxMbMZU-8rjqom-MV=ZknGpJib86MQPhg=4wMPZPA at mail dot gmail dot com> <CABtf2+TkhDf-j=0TYCGFS6q7b4EbsreyCpqEqdNkQ8+Ho1ojww at mail dot gmail dot com> <CABtf2+SCRfAyOHpqyDPPWUh_FaMFgGAVk4sT=opu6grBGfemEQ at mail dot gmail dot com> <CABtf2+S1+mLLT0By41to0D-BELAM1ZD=WEu_9cQi6SSAuBiCCQ at mail dot gmail dot com> <521E33CE dot 7020101 at google dot com>
On Wed, Aug 28, 2013 at 10:30 AM, Diego Novillo <email@example.com> wrote:
> On 2013-08-28 12:59 , Caroline Tice wrote:
>> static void
>> -output_set_info (tree record_type, tree *vtbl_ptr_array, int array_size)
>> +output_set_info (tree record_type, vec<tree> *vtbl_ptr_array)
Okay, will do.
> Since this function does not modify vtbl_ptr_array, you could pass it by
>> @@ -1023,23 +1010,23 @@ register_all_pairs (tree body)
>> if (flag_vtv_debug)
>> output_set_info (current->class_info->class_type,
>> - vtbl_ptr_array, num_vtable_args);
>> + vtbl_ptr_array);
>> - if (num_vtable_args > 1)
>> + if (vtbl_ptr_array->length() > 1)
>> - insert_call_to_register_set (current->class_name,
>> + insert_call_to_register_set (current->class_name,
>> vtbl_ptr_array, body, arg1, arg2,
>> registered_at_least_one = true;
>> - else if (num_vtable_args >= 0)
>> + else
num_vtable_args was an int that we manually incremented/decremented,
so it might have been negative. I am
assuming that a vec<tree>.length() can never be negative. So before
the else-clause was explicitly checking that the value was 0 or 1
(since the if-condition takes all values greater than 1. Now I am
implicitly assuming that when we get to the else-clause the value must
be 0 or 1, because the if-condition took care of all values greater
than 1, and values less that zero cannot occur. Is that assumption
Do you need to review this again after I fix your suggestion, or can I
go ahead and commit it after?
> You've changed the meaning of this else now. Intended? I can't tell from
> The patch looks fine, otherwise.