stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Travis Vitek" <Travis.Vi...@roguewave.com>
Subject RE: implementation of Unary Traits
Date Mon, 30 Jun 2008 22:26:40 GMT
 

Martin Sebor wrote:
>
>Travis Vitek wrote:
>>  
>> 
>> Martin Sebor wrote:
>>> The implementation of Unary Traits (e.g., is_void) uses explicit
>>> specialization on all four combinations of cv-qualifiers for each
>>> trait (plain, const, volatile, and const volatile). I'm wondering
>>> if the alternative approach of stripping the qualifiers before
>>> "dispatching" to just one explicit specialization has been
>>> considered. 
>> 
>> That is what I had originally done.
>
>I forgot about that.
>
>[...]
>> 
>> The only issue is that this creates a cyclic dependency 
>> between traits
>> defined in <rw/_meta_cv.h> and those in <rw/_meta_cat.h>.
>
>But only because of the __rw_add_xxx Transformation Traits. Not
>because of the __rw_is_xxx Unary Traits, correct?

Yes.

>IMO, the two
>categories belong in separate headers anyway. Not just logically
>but also (and mainly) because unlike the latter, I don't expect
>us to be needing the former in [many] other areas of the library.

I can accept that.

Originally I wanted separate headers for each trait, but it was
determined that the overhead from including all of these small files
would be to much, so I suggested splitting traits up into groups based
on what they did (<rw/_add_cv.h>, <rw/_remove_cv.h>, ...) but that was
vetoed also.

>
>> An easy way to
>> 'fix' this would be to forward declare __rw_remove_cv in
>> <rw/_meta_cat.h> and then include <rw/_meta_cv.h> at the bottom, but
>> this goes against our conventions. Another option was to put the
>> remove-cv traits into their own header, but this went against your
>> request to organize the traits in files according to some rationale.
>
>My suggested guideline is to group traits according to how likely
>they are to be used in other areas of the library.

It seems that your 'suggested guideline' is in direct conflict with your
'requirement' that the traits be arranged into files according to some
'well defined rationale'. This is the same reasoning you used to reject
my proposal of splitting related traits into separate files.

>From what I gather, you are suggesting that all traits go into
<type_traits> unless they are likely to be used in other areas of the
library. If they are 'likely' to be used in other parts of the library,
then they go into some other header(s), the names of which depend on
some yet to be determined naming scheme. So the 'well defined rationale'
does not seem to be very well defined.

>I expect the
>is_cv-kind of traits to be used pervasively (in containers and
>some algorithms). OTOH, I expect the add_cv ones to be used only
>exceedingly rarely, if ever. Traits that are not used in other
>library headers can be defined directly in <type_traits> or
>grouped in implementation-specific headers according to some
>other sensible criteria.

Okay, I've already taken two shots at this. One that I showed as an
initial implementation for review, and the other which has been
committed to subversion.

>I don't have a strong preference as
>long as they don't get pulled in to translation units
>unnecessarily.

This is in conflict with the feedback you provided on my initial traits
implementation. You didn't like that each trait was in its own header,
yet no trait was unnecessarily pulled in.

>
>> 
>>> The potential advantage of this approach is fewer
>>> declarations, smaller translation units, and thus (presumably)
>>> faster compilation.
>>>
>> 
>> There has to be a balance somewhere.
>
>Absolutely.
>
>> If all traits are in one file, we
>> have few includes, but lots of unnecessary declarations. If we spread
>> them all out, then we have few unnecessary declarations, but many
>> includes. Both can _potentially_ lead to (ever so slightly) longer
>> compile times in some cases and shorter ones in other cases.
>> 
>> I'm definitely open to suggestions and I'm willing to make 
>any necessary
>> changes. Unfortunately any suggestion has to take the 
>following criteria
>> into consideration...
>> 
>>   A. the number of trait headers should be kept to a minimum (to keep
>> compile times down)
>>   B. there should not be many unnecessary declarations in 
>any header (to
>> keep compile times down)
>>   C. traits should be put into files according to some well defined
>> rationale (to keep things organized)
>> 
>> Unfortunately, I don't really see a clear path to satisfying 
>> all of the above requirements.
>
>I can think of a couple of options that satisfy these. I'm not sure
>how palatable I find them yet, but I might get over their (initial)
>lack of appeal if the payoff is worth it. YMMV.
>
>One is to drop the cv specializations of the __rw_is_xxx traits and
>define the standard traits in terms of both remove_cv and the __rw
>traits, like so:
>
>   template <class _TypeT>
>   struct is_void:
>       integral_constant<bool,
>           _RW::__rw_is_void<typename
>               _RW::__rw_remove_cv<_TypeT>::type>::value>
>   { };
>
>or more concisely:
>
>   template <class _TypeT>
>   struct is_void:
>       integral_constant<bool, _RWSTD_IS_VOID (_TypeT)::value>
>   { };
>
>with _RWSTD_IS_VOID() being #defined like so:
>
>   #define _RWSTD_IS_VOID(T) \
>       _RW::__rw_is_void<typename _RW::__rw_remove_cv<T>::type>
>

If you do it this way, __rw_is_void<> only tells you if a type is void
or not, it doesn't tell you if the type is a possibly cv-qualified void.
I suppose that this is fine, provided you never use the __rw_is_void<>
template directly.

This also seems to require that we keep the _RWSTD_TT_* macros because
we always need to define the template types for use with the macro.
I.e., you will run into serious problems if you attempt to do something
like

  #define _RWSTD_HAS_NOTHROW_CTOR(T) \
      __has_nothrow_default_ctor(T)

  template <class _TypeT>
  struct has_nothrow_default_ctor:
      integral_constant<bool, _RWSTD_HAS_NOTHROW_CTOR (_TypeT)::value>
  { };


>All internal code would always need to use _RWSTD_IS_VOID() and
>never _RW::__rw_is_void directly.

If this is the case, then I see no motivation for
__rw_integral_constant<>.

>The other is to introduce an additional intermediate template for
>each of the __rw_is_xxx traits to remove the cv qualifiers:
>
>     template <class _TypeT>
>     struct __rw_is_void_impl: __rw_false_type { };
>
>     template <>
>     struct __rw_is_void_impl<void>: __rw_true_type { };
>
>     template <class _TypeT>
>     struct __rw_is_void:
>         __rw_is_void_impl<typename __rw_remove_cv<_TypeT>::type>
>     { };
>
>The first alternative is definitely the cheapest in terms of lines
>of new code required to implement it, but I know that not everyone
>is a fan of macros. The redeeming fact is that IIRC we were planning
>to use the _RWSTD_IS_XXX() macros anyway, so this change shouldn't
>be a problem.
>
>The second one seems somewhat "cleaner" but I suspect adding the
>additional template might cancel out the savings gained by removing
>the explicit specialization.
>
>Let me know what you think.

If you ask what I prefer, I'm going to tell you I prefer the second
option (that is essentially what I wrote originally). But, honestly, for
me to care either way, I need to know that there actually a noticeable
performance difference between the two techniques.

>
>Martin
>

Mime
View raw message