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