incubator-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: svn commit: r667365 [1/3] - in /stdcxx/branches/4.3.x: etc/config/src/ include/ include/rw/ tests/utilities/
Date Mon, 16 Jun 2008 17:47:43 GMT
 

Martin Sebor wrote:
>
>vitek@apache.org wrote:
>> Author: vitek
>> Date: Thu Jun 12 22:53:51 2008
>> New Revision: 667365
>
>I understand that this is work in progress so just to help
>make progress here are a few comments on some small issues
>with this change:
>
>Some files are missing the required terminating newline (they
>are noted in the svn change notification).

Fixed in http://svn.apache.org/viewvc?rev=668208&view=rev.

>Also, some of the #include guards are inconsistent with the
>names of the headers they guard after their renaming. The
>guards need to be reviewed and bring into harmony with the
>names (we night want to write a simple shell script to help
>with this process).

Fixed in http://svn.apache.org/viewvc?rev=668223&view=rev.

>
>Finally, I'm not sure the patch is consistent WRT the use
>of the type and value (vs. _C_type and _C_value) typedefs.
>I hope we can agree on using of the public names throughout.

I opted to use the prefix for helpers that should never be anywhere
except in the implementation of the actual trait. For example,
__rw_add_lvalue_reference_impl<> uses '_C_type' and
__rw_add_lvalue_reference<> uses 'type'.

>
>> 
>> URL: http://svn.apache.org/viewvc?rev=667365&view=rev
>> Log:
>> 2008-06-12  Travis Vitek  <vitek@roguewave.com>
>> 
>> 	STDCXX-916
>> 	* include/type_traits: New file defines C++0x type traits.
>> 	* include/rw/_config-gcc.h: Add macros for compiler support.
>
>The macro names should be listed here so we can find them in
>ChangeLogs.

Log updated.

>I was also hoping we were in agreement on removing
>the _TT_ part from the macro names.

I do recall that you suggested this, but I don't recall agreeing to
this.

One disadvantage of removing them is that the macros are defined in one
file if the trait has compiler support or in another file if it doesn't.
i.e. _RWSTD_IS_ENUM() is defined if you #include rw/_defs.h if compiler
support is available, but it would be defined in rw/_meta_cat.h if it is
not. This could lead to failures because the 'user' forgot to include
the correct header and tested on platforms that had compiler support.

>
>> 	* include/rw/_config-msvc.h: Ditto.
>> 	* include/rw/_config.h [_RWSTD_EXT_CXX_OX]: Disable C++0x
>> 	extensions unless defined.
>> 	* include/rw/_defs.h [_RWSTD_SWCHAR_INT_T]: Add new macro that
>> 	defines a type that has same size and layout a the fictional
>> 	signed wchar_t.
>
>What do we need _RWSTD_SWCHAR_INT_T for?
>
>[...searching...]
>
>I see: for make_signed. Are you sure the traits are correct for
>char and wchar_t? Also, do you believe the working draft to be
>unambiguous and correct? Based on my reading, make_signed<T>
>::type should be the same as T if T is a signed integral type.
>IIUC, our make_signed<char>::type is defined to signed char
>regardless of the signed-ness of char. Ditto for wchar_t.
>
>Also, I assume the big block of #ifdef'ed out code with the
>_RWSTD_TT_XXX() macro definitions was meant to be deleted?
>
>Finally, the (unsafe) change to _RWSTD_UWCHAR_INT_T isn't
>mentioned here.

I'll update the log but I'm unsure exactly why this change is unsafe.

>It should be reverted (and _RWSTD_SWCHAR_INT_T
>should be changed to something like _RWSTD_PTRDIFF_T or maybe
>_RWSTD_SSIZE_T.

I don't understand why this is any better. It would seem that if there
are a finite number of [un]signed integral types and we want a
[un]signed integral type that is the same size as 'wchar_t' we would
just look at the [un]signed integral types. There should be no need to
even consider anything else.

If anything I should have added a #error case if _RWSTD_WCHAR_SIZE is
not equal to the size of at least one of the integral types.

>
>Martin
>

Mime
View raw message