On 2012-10-17 06:14 , Richard Biener wrote:
Can't we implement vec_e as vec<element_type, embedded> by
means of a partial specialization?
Ah, yes, that's certainly possible and prevents renaming the types in
the future.
I have not yet implemented the "fast" vectors from my proposal.
Those would be useful for free variables or structures that can
tolerate an extra word of storage.
Needless to say the patch is gigantic: 2.4 Mb, 334 files
changed, 10718 insertions(+), 11536 deletions(-). But it is
largely mechanic.
We no longer access the vectors via VEC_* macros. The pattern is
"VEC_operation (T, A, V, args)" becomes "V.operation (args)".
That is mostly why you see ~800 fewer lines in the patch.
The only thing I could not do is create proper ctors and dtors
for the vec_s/vec_e classes. Since these vectors are stored in
unions, we have to keep them as PODs (C++03 does not allow
non-PODs in unions).
This means that creation and destruction must be explicit. There
is a new method vec_s<type, allocation>::create() and another
vec_s<type, allocation>::destroy() to allocate the internal
vector.
::alloc() and ::free()? I see you have those, too, but from
/* Vector allocation. */
static vec_s<T, A> *alloc (unsigned CXX_MEM_STAT_INFO);
static void free (vec_s<T, A> *&);
/* Internal vector creation and initialization. */
static vec_s<T, A> create (unsigned CXX_MEM_STAT_INFO);
static vec_s<T, A> create (unsigned, vec_e<T> *);
void destroy (void);
I can't see how they differ and what users should use? I suppose
::create and ::destroy are just implementation details? Thus
you'd make them private? Why is create not a member function?
It should be IMHO
This is the part I don't like too much. The problem is that we can't
use ctors/dtors here. Both ::alloc() and ::free() allocate a pointer to
a vec. So they can't really be member functions. We don't really have
any calls to ::alloc/::free now (I added them for completeness), so
leaving them out for now is certainly possible.
::create is special too not because it needs to allocate a pointer, but
because it looked odd when used as a regular member function. Consider:
foo(unsigned n)
{
vec_s<tree> v = v.create(n);
...
}
We are calling ::create on an "uninitialized" object. This will work,
because ::create() allocates the internal vector and reads no state from
'v'. It just looked odd. Ideally, this code would look like:
foo(unsigned n)
{
vec<tree> v(n);
...
}
But we can't have constructors on vec (I'm tempted to add a vector class
that cannot be put in unions but does provide ctors/dtors).
With the current syntax, we do:
foo(unsigned n)
{
vec<tree> v = vec<tree>::create(n);
}
This has the problem of being overly verbose (particularly when the
template arguments are long). So, if we don't mind the slightly odd
syntax in the first version, I could just change ::create to be a
regular member function.
template<typename T, enum vec_allocation A>
inline vec_s<T, A>&
vec_s<T, A>::create (unsigned nelems, vec_e<T> *space)
{
gcc_assert (A == va_stack);
this->stack_reserve (space, nelems);
return *this;
}
Otherwise please use gcc_checking_assert in inline functions,
otherwise they won't be inlined due to size constraints in most
of the cases.
Good point. Thanks.
Likewise ::copy should mimic a copy constructor. Not sure if
that will work out ... isn't it enough that vec_e is embeddable
into a union?
You want to make ::copy a static function? Not sure I'm following you here.
Otherwise thanks for working on this and sorry for the C++
bikeshedding - there are just so many more ways to do something
in C++ compared to C ;)
Heh, indeed. Thanks for the review.
Diego.
