Re: [PATCH v2 09/13] expression: examine constness of __builtin_offsetof at evaluation only

[Nicolai Stange <nicstange@gmail.com>]

Luc Van Oostenryck <luc.vanoostenryck@gmail.com> writes:

> On Mon, Jan 25, 2016 at 04:00:48PM +0100, Nicolai Stange wrote:
>> Currently, the determination of a __builtin_offsetof() expressions'
>> constness flags is done in two steps:
>> - Several flags are speculatively set at expression parsing time
>> - and possibly cleared again at evaluation if the member expression
>>   includes a non-const array index like in
>>     __builtin_offsetof(struct A, a.b[non_const_foo])
>> 
>> For consistency with other expression types' evaluation, defer the
>> determination of a __builtin_offsetof() expression's constness to
>> evaluation time, too.
>> 
>> Furthermore, carry an array index expression's constness flags
>> through the implicit cast to size_t type.
>
> Better to split this into two patches.
>  
>> @@ -3028,13 +3026,18 @@ static struct symbol *evaluate_offsetof(struct expression *expr)
>>  		} else {
>>  			struct expression *idx = expr->index, *m;
>>  			struct symbol *i_type = evaluate_expression(idx);
>> +			unsigned old_idx_flags;
>>  			int i_class = classify_type(i_type, &i_type);
>> +
>>  			if (!is_int(i_class)) {
>>  				expression_error(expr, "non-integer index");
>>  				return NULL;
>>  			}
>>  			unrestrict(idx, i_class, &i_type);
>> +			old_idx_flags = idx->flags;
>>  			idx = cast_to(idx, size_t_ctype);
>> +			idx->flags |= old_idx_flags;
>> +			expr_flags_decay_consts(&idx->flags);
>>  			m = alloc_const_expression(expr->pos,
>>  						   bits_to_bytes(ctype->bit_size));
>>  			m->ctype = size_t_ctype;
>
> It's not clear at all to me why this is needed.
> Why cast_to() can't set itself the right value for idx->flags?

cast_to() is for implicit casts and to be honest, I don't know what the
->flags should be set to.

Also, this is the only place in the whole code where preserving the
flags over a cast_to() call is needed.

Furthermore, we could get rid of the flags' save and restore by moving
the calculation of expr->flags in front of the cast_to() invocation --
all other cast_to() actually do it this way.

However, I think, it's better to have the assignment to expr->flags
grouped with the rest of the assignments to expr's members.