On 5/24/12, Diego Novillo <dnovi...@google.com> wrote:
> On 12-05-24 04:16 , Richard Guenther wrote:
>> On May 23, 2012 Diego Novillo<dnovi...@google.com> wrote:
>>> Some client code changes were needed:
>>>
>>> 1- The allocation names 'heap', 'stack' and 'gc' are not embedded in
>>> the type name anymore. They are enum values used as a template
>>> argument for functions like VEC_alloc() and VEC_free(). Since they
>>> are now separate identifiers, some existing code that declared
>>> variables with the names 'heap' and 'stack' had to change.
>>>
>>> I did not change these enum values to better names because that
>>> would have meant changing a lot more client code. I will change
>>> this in a future patch.
>>
>> Yeah. Can we have
>>
>> template<...>
>> struct vec {
>> enum { gc, heap, stack };
>> ...
>> }
>>
>> and use vec<T, vec::stack> instead? Not sure if this or something
>> similar
>> is valid C++.
>
> Sure. Something involving a new namespace, like Gaby suggested
> down-thread.
I like namespaces, but "using namespace" does not work the way most
people think it does. So, I was planning to defer that issue for
a while.
>>> 2- VEC_index and VEC_last now return a reference to the element. The
>>> existing APIs implemented two versions of these functions. One
>>> returning the element itself (used for vec_t<T *>). Another
>>> returning a pointer to the element (used for vec_t<T>).
>>>
>>> This is impossible to represent in C++ directly, as functions
>>> cannot be overloaded by their return value. Instead, I made them
>>> return a reference to the element. This means that vectors storing
>>> pointers (T *) do not need to change, but vectors of objects (T) do
>>> need to change.
>>>
>>> We have fewer vec_t<T> than vec_t<T *>, so this was the smaller
>>> change (which still meant changing a few hundred call sites).
>>
>> We still can have const overloads when taking a const vec though?
>
> I'm going to say 'sure' because I don't really think I understand this
> question :)
Yes, we can.
>>> 3- The family of VEC_*push and VEC_*replace functions have overloads
>>> for handling 'T' and 'const T *' objects. The former is used for
>>> vec_t<T>, the latter for vec_t<T*>. This means, however, than when
>>> pushing the special value 0 on a vec_t<T*>, the compiler will not
>>> know which overload we meant, as 0 matches both. In these cases
>>> (not very many), a static cast is all we need to help it.
>>>
>>> I'm not terribly content with this one. I'll try to have a better
>>> approach in the second iteration (which will liberally change all
>>> client code).
>>
>> I wonder whether explicitely specifying the template type is better?
>> Thus, VEC_safe_push<T *, gc>(0) instead of automatically deducing the
>> template argument?
>
> Not sure. I didn't like the casts, Lawrence had another suggestion
> involving something like C++11's nullptr. Lawrence, what was that
> nullptr thing you were telling me earlier?
One would write VEC_save_push(v, nullptr), which unambiguously
means the null pointer. C++11 has nullptr built in to the language,
but we can approximate it with:
struct nullptr_t {
template <typname T>
operator T*() { return 0; }
} nullptr;
The downside is that using plain 0 would still be ambiguous.
Programmers that mean the integer would need to cast the 0.
>>> 4- I extended gengtype to understand templated types. These changes
>>> are not as ugly as I thought they would be. Gengtype has some
>>> hardwired knowledge of VEC_*, which I renamed to vec_t. I'm not
>>> even sure why gengtype needs to care about vec_t in this way, but I
>>> was looking for minimal changes, so I just did it.
>>>
>>> The other change is more generic. It allows gengtype to deal with
>>> templated types. There is a new function (filter_type_name) that
>>> recognizes C++ special characters '<','>' and ':'. It turns them
>>> into '_'. This way, gengtype can generate a valid identifier for
>>> the pre-processor. It only does this in contexts where it needs a
>>> pre-processor identifier. For everything else, it emits the type
>>> directly. So, the functions emitted in gt-*.h files have proper
>>> template type references.
>>
>> Well, that part is of course what needs to change. I don't think we want
>> gengtype to work like this for templates as this does not scale.
>
> Well, certainly, but that was outside the scope of this one patch. I'm
> trying to make small incremental steps.
>
>>> 2- Change VEC_index into operator[].
>>>
>>> 3- Change VEC_replace(...) to assignments using operator[].
>>
>> I think these should not be done for now - they merely add syntactic
>> sugar, no?
>
> It makes the code more compact, readable and easier to write.
> I can certainly wait until the branch has been merged to make
> all these changes. It's going to affect a boatload of call sites.
How about picking a one or two files to show the effect, but leave
the bulk of the call site changes to later changes directly on trunk?
--
Lawrence Crowl
