Re: [PATCH] Cilk Plus Array Notation for C++

[Aldy Hernandez <aldyh at redhat dot com>]

> > Overall, a lot of the stuff in cp-array-notation.c looks familiar
> > from the C front- end changes.  Can't you reuse a lot of it?
>
> I looked into trying to combine many functionality. The issue that
> prohibited me was templates and extra trees. For example, IF_STMT,
> FOR_STMT, MODOP_EXPR, etc are not available in C but are in C++. So,
> I had to add this additional check for those. Also, if we are
> processing templates we have to create different kind of trees (e.g
> MODOP_EXPR intead of MODIFY_EXPR).

I see.

> One way to do it is to break up the places where I am using C++
> specific code and add a language hook to handle those. I tried doing
> that a while back and the whole thing looked a lot messy and I would
> imagine it would be hard to debug them in future (...atleast for me).
> This looked organized for me, even though a few code looks repeated.

That's what I had in mind, but if you tried it and it looks worse, I 
guess I can live with it.

> > You do all this type checking here, but aren't you doing the same
> > type checking in build_array_notation_ref() which you're going to
> > call anyway?  It looks like there is some code duplication going
> > on.
>
> The reason why we do this second type checking here is because we
> don't know what they could be when we are parsing it. For example,
> in:

Couldn't you abstract the type checking out into a helper function 
shared by both routines?

> > Also, I see you have a build_array_notation_ref() in
> > cp/cp-array-notation.c and also in c/c-array-notation.c.  Can you
> > not implement one function that handles both C and C++, or at the
> > very least reuse some of the common things?
>
> I looked into that also, but templates got in the way.

Ughh... ok, I'll let Jason deal with this then.

> > > +  XDELETEVEC (compare_expr); +  XDELETEVEC (expr_incr); +
> > > XDELETEVEC (ind_init); +  XDELETEVEC (array_var); + +  for (ii =
> > > 0; ii < list_size; ii++) +    { +      XDELETEVEC
> > > (count_down[ii]); +      XDELETEVEC (array_value[ii]); +
> > > XDELETEVEC (array_stride[ii]); +      XDELETEVEC
> > > (array_length[ii]); +      XDELETEVEC (array_start[ii]); +
> > > XDELETEVEC (array_ops[ii]); +      XDELETEVEC
> > > (array_vector[ii]); +    } + +  XDELETEVEC (count_down); +
> > > XDELETEVEC (array_value); +  XDELETEVEC (array_stride); +
> > > XDELETEVEC (array_length); +  XDELETEVEC (array_start); +
> > > XDELETEVEC (array_ops); +  XDELETEVEC (array_vector);
> >
> > I see a lot of this business going on.  Perhaps one of the core
> > maintainers can comment, but I would rather use an obstack, and
> > avoid having to keep track of all these little buckets-- which
> > seems rather error prone, and then free the obstack all in one
> > swoop.  But I'll defer to Richard or Jason.
>
> They are temporary variables that are used to store information
> necessary for expansion. To me, dynamic arrays seem to be the most
> straight-forward way to do it. Changing them would involve pretty
> much rewriting the whole thing and thus maybe breaking the stability.
> So, if it is not a huge issue, I would like to keep the dynamic
> arrays. They are not being used anywhere else just inside the
> function.

This is not huge, so don't worry, but XNEWVEC is just a wrapper to 
xmalloc (see include/libiberty.h).  You could do the exact thing with 
XOBNEWVEC and save yourself all the XDELETEVECs, with one obstack_free().