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.
