Re: [LTO][PATCH] Supporting multiple lto_out_decl_states.

["=?big5?b?RG91ZyBLd2FuICjD9q62vHcp?=" <dougkwan at google dot com>]

There was a coliision between my patch and Rafael's.  He beated me in
submitting :)  Now I need to adjust my patch.  One thing I noticed was
that there were new failures since updating my sandbox and my
top-of-branch tree to include Rafael's symtab patch.  I am now looking
into that but I am sick today so I am not working in my full capacity.

-Doug

2008/8/19 Diego Novillo <dnovillo@google.com>:
> On 8/5/08 6:01 PM, Doug Kwan (Ãö®¶¼w) wrote:
>>
>> This patch adds support multiples lto_out_decl_states, which is
>> required by the WPA since it writes multiple output files.  It has
>> been tested on i686-unknown-linux-gnu and had not new regression.
>
> For future patches (particularly big patches like this one), it's very
> helpful to include a general description of how the patch works.  This helps
> a lot during reviewing to give some context to the patch.
>
>> +/* Return a new lto_out_decl_state which is a child of PARENT. */
>> +
>
> What is PARENT?  Were you going to organize the states in some tree fashion?
>
>
>> +struct lto_out_decl_state *
>> +lto_new_out_decl_state (void)
>> +{
>> +  struct lto_out_decl_state *state;
>> +  int i;
>> +
>> +  state = ((struct lto_out_decl_state *)
>> +          xcalloc (1, sizeof (struct lto_out_decl_state)));
>
> s/xcalloc/XNEW/
>
>> +struct lto_out_decl_state *
>> +lto_pop_out_decl_state (void)
>
> Needs comment.
>
>
>> +enum {
>> +  LTO_DECL_STREAM_FIELD_DECL = 0,
>> +  LTO_DECL_STREAM_FN_DECL = 1,
>> +  LTO_DECL_STREAM_VAR_DECL = 2,
>> +  LTO_DECL_STREAM_TYPE_DECL = 3,
>> +  LTO_DECL_STREAM_NAMESPACE_DECL = 4,
>> +  LTO_DECL_STREAM_TYPE = 5,
>> +  LTO_N_DECL_STREAMS = 6
>> +};
>
> No real need to give values to the symbols after 0, but that's OK.
>
>
>
> The rest is OK.  Nice cleanup.
>
>
> Thanks.  Diego.
>
>