RE: Patch for Bug No-8081

["Sitikant Sahu, Noida" <sitikants at noida dot hcltech dot com>]

Hello,

Zack wrote,

>> The space is allocated in the caller. The expand_call () is being 
>> processed for the caller. The values within the returned block can >> be
accessed if we write something like
>>     func ().val[i]

>Have you verified that you get a null target in expand_call for this
>construct too?  Your test case doesn't do that.

Yes,for this construct also we get a null target in expand_call and the
program flows through the fix only.


>> Am I missing something, if thats not the case ?

>I misunderstood the code.  This is indeed being executed in the
>caller.  So that isn't a problem, and I agree that allocating dynamic
>stack space at this point is the right thing to do in the abstract.

>However, I think there's still a problem here - there wasn't anything
>wrong with the code you replaced, it was just being run in the wrong
>set of circumstances.  What the old comment is trying to say is, "This
>block of code only works for fixed size structure types.  This is fine
>because we can't ever get here with a variable sized type because our
>caller should have given us a target."  Which, it turns out, is not 
>true.

>So what you actually want to do is

>       ...
>      else if (target && GET_CODE (target) == MEM)
>         structure_value_addr = XEXP (target, 0);
>       else if (variably_modified_type_p (TREE_TYPE (exp))
>         /* your new code here */
>       else
>        /* old code here */

>The reason this matters is, using assign_stack_temp when it'll work
>(i.e. for a fixed size structure) produces much better code than
>always using allocate_dynamic_stack_space.

   Actually, when the size of the structure is fixed, it also doesn't flow
through the old code either.In that case it flows through another path.The
old code here will be like a dead code.So I believe it is better not to have
the old code at this position. 


>In comments, you should not talk about bogus assumptions made by code
>that no longer exists.  Comments should document the code as it is
>now.  I suggest you say only

> /* If the return type is variably modified and we have no target to
>    initialize, we must use allocate_dynamic_stack_space to allocate a
>    temporary.  It becomes inaccessible at the end of the surrounding
>    expression, so its lifetime does not matter (it will be at least
>    that long).  */

Ok, I'll include this comment rather than specifying earlier assumptions.

>Except, is that statement about its lifetime true?  There are a number
>of constructs that could be used to extend the life of the object,
>such as 

>  int *p = &func().val[i];

>Bleah ill-thought-out extensions...

Here the C parser takes care of semantics and emits the error.  

Thanks,
Sitikant sahu