On Fri, Aug 09, 2024 at 04:46:55PM +0100, Alex Coplan wrote:
> On 09/08/2024 17:34, Jakub Jelinek wrote:
> > On Fri, Aug 09, 2024 at 04:21:14PM +0100, Alex Coplan wrote:
> > > Hmm, good spot, I didn't realise that convert_from_reference could
> > > change the type of the condition like this.
> > >
> > > In that case I suppose the only thing to do is to construct a stack of
> > > ANNOTATE_EXPRs on the way down and re-build the expressions (taking the
> > > type of the inner expression, starting with the cond) on the way back
> > > up.
> >
> > I think you don't need to rebuild them, just update their types.
> > Something along the lines of
> > for (tree c = cond; c != *condp; c = TREE_OPERAND (c, 0))
> > {
> > gcc_checking_assert (TREE_CODE (c) == ANNOTATE_EXPR);
> > TREE_TYPE (c) = TREE_TYPE (*condp);
> > }
>
> I suppose I was just concerned about any other properties of
> ANNOTATE_EXPRs that might be inherited from the operand (that could be
> affected by such a change).
>
> It looks like TREE_{READONLY,THIS_VOLATILE,SIDE_EFFECTS} are all set
> in convert_from_reference and propagated by build3, so if those change
> underneath us then only updating the type seems insufficient.
I think TREE_THIS_VOLATILE isn't, that is only for references.
The others could change, but only in the !TREE_SIDE_EFFECTS ->
TREE_SIDE_EFFECTS or TREE_READONLY -> !TREE_READONLY direction
(the former if it was volatile bool &).
So you could also in the loop update it just in case,
TREE_SIDE_EFFECTS (c) |= TREE_SIDE_EFFECTS (*condp);
TREE_READONLY (c) &= TREE_READONLY (*condp);
Jakub
