Re: Re-implement VEC_* to be member functions of vec_t<T>

[Richard Guenther <rguenther at suse dot de>]

On Thu, 23 Aug 2012, Diego Novillo wrote:

> This patch is the first step towards making the API for VEC use
> member functions.
> 
> There are no user code modifications in this patch.  Everything
> is still using the VEC_* macros, but this time they expand into
> member function calls.
> 
> Because of the way VECs are used, this required some trickery.
> The API allows VECs to be NULL.  This means that services like
> VEC_length(V) will return 0 when V is a NULL pointer.  This is,
> of course, not possible to do if we call V->length().
> 
> For functions that either need to allocate/re-allocate the
> vector, or they need to handle NULL vectors, I implemented them
> as static member functions or free functions.
> 
> Another wart that I did not address in this patch is the fact
> that vectors of pointers and vectors of objects have slightly
> different semantics when handling elements in the vector.  In
> vector of pointers, we pass them around by value, but in vectors
> of objects, they are passed around via pointers.  That's why we
> need TYPE * and TYPE ** overloads for some functions (e.g.,
> vec_t::iterate).
> 
> I will fix these two warts in a subsequent patch.  The idea is to
> make vec_t a single-word structure, which acts as a handler for
> the structure containing the actual vector.  Something like this:
> 
> template<typename T>
> struct vec_t
> {
>   struct vec_internal<T> *vec_;
> };
> 
> This has the advantage that we can now declare the actual vector
> instances as regular variables, instead of pointers.  They will
> use the same amount of memory when embedded in other structures,
> and we will be able to allocate and reallocate the actual data
> without having to mutate the vector instance.
> 
> All the functions that are now static members in vec_t, will
> become instance members in the new vec_t.  This will mean that
> all the callers will need to be changed, of course.
> 
> There is another issue that I need to address and I'm not quite
> sure how to go about it: with the macro-based API, we make use of
> pre-processor trickery to insert __FILE__, __LINE__ and
> __FUNCTION__ into the argument list of functions.
> 
> When I change VEC_pop(V) with V->pop(), the macro expansion no
> longer exists and we lose the caller references.  Richi, I
> understand that your __builtin_FILE patch would allow me to
> declare default values for these arguments? Something like:
> 
> T vec_t<T>::pop(const char *file_ = __builtin_FILE,
> 		unsigned line_ = __builtin_LINE,
> 		const char *function_ = __builtin_FUNCTION)
> 
> which would then be evaluated at the call site and get the right
> values.  Is that more or less how your solution works?

Yes.  I'll pick up on this patch again after I recovered from
my vacation.

> If so, then we could get away with that in most cases.  However,
> we would still have the problem of operator functions (e.g.,
> vec_t::operator[]).
> 
> I think I would like to explore the idea of implement a stack
> unwinder that's used by gcc_assert().  This way: (a) we do not
> need to uglify all the APIs with these extra arguments, (b) we
> can control how much of the call stack we show on an assertion.
> 
> Would that be something difficult to implement?  I don't think we
> need something as generic as libunwind.  Thoughts?

It won't work and it is not portable.  So I don't think we want to
go that way.  operator[] does never do allocation so you won't need
the file/line/function arguments.

Richard.