incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: svn commit: r672395 - in /stdcxx/branches/4.3.x/include: functional rw/_ref_wrap.h
Date Sat, 28 Jun 2008 20:51:14 GMT
elemings@apache.org wrote:
> Author: elemings
> Date: Fri Jun 27 14:10:30 2008
> New Revision: 672395
> 
> URL: http://svn.apache.org/viewvc?rev=672395&view=rev
> Log:
> 2008-06-27  Eric Lemings <eric.lemings@roguewave.com>
> 
> 	STDCXX-958
> 	* include/rw/_ref_wrap.h: Convert internal namespace members to
> 	standard namespace members (at least until such a time when
> 	internal namespace members are actually needed).
> 	(reference_wrapper): Implemented basic constructors, operators,
> 	and accessors.  (No specializations, function inheritance,
> 	result_of, or invocation operators yet.)
> 	(ref, cref): Also implemented.
> 	* include/functional: Include <rw/_ref_wrap.h> header.
> 	(Internal header contains C++0x guards so it only gets compiled
> 	for C++0X code.)


I assume this is still work in progress, so just a few general
comments regarding convention and style intended to help reduce
the amount of cleanup work that will eventually need to be done
before committing the final version of the code.

> 
> 
> Modified:
>     stdcxx/branches/4.3.x/include/functional
>     stdcxx/branches/4.3.x/include/rw/_ref_wrap.h
> 
> Modified: stdcxx/branches/4.3.x/include/functional
> URL: http://svn.apache.org/viewvc/stdcxx/branches/4.3.x/include/functional?rev=672395&r1=672394&r2=672395&view=diff
> ==============================================================================
> --- stdcxx/branches/4.3.x/include/functional (original)
> +++ stdcxx/branches/4.3.x/include/functional Fri Jun 27 14:10:30 2008
> @@ -37,7 +37,7 @@
>   * permissions and limitations under the License.
>   *
>   * Copyright 1994-2006 Rogue Wave Software.
> - * 
> + *
>   **************************************************************************/
>  
>  #ifndef _RWSTD_FUNCTIONAL_INCLUDED
> @@ -45,10 +45,11 @@
>  
>  
>  #include <rw/_funcbase.h>
> +#include <rw/_ref_wrap.h>

The #include directive should be guarded by #ifndef
_RWSTD_NO_EXT_CXX_0X. That way non-C++ 0x users won't be penalized
by #including the header and we won't have to worry about wrapping
the whole contents of <rw/_ref_wrap.h> in a pair of these #ifdef/
#endif directives.


>  #include <rw/_defs.h>
>  
>  
> -_RWSTD_NAMESPACE (std) { 
> +_RWSTD_NAMESPACE (std) {
>  
>  
>  // 20.3.2 - Arithmetic operations
> @@ -663,7 +664,7 @@
>  }   // namespace std
>  
>  
> -_RWSTD_NAMESPACE (__rw) { 
> +_RWSTD_NAMESPACE (__rw) {
>  
>  
>  // extension: returns the argument
> 
> Modified: stdcxx/branches/4.3.x/include/rw/_ref_wrap.h
> URL: http://svn.apache.org/viewvc/stdcxx/branches/4.3.x/include/rw/_ref_wrap.h?rev=672395&r1=672394&r2=672395&view=diff
> ==============================================================================
> --- stdcxx/branches/4.3.x/include/rw/_ref_wrap.h (original)
> +++ stdcxx/branches/4.3.x/include/rw/_ref_wrap.h Fri Jun 27 14:10:30 2008
> @@ -24,8 +24,8 @@
>   * implied.   See  the License  for  the  specific language  governing
>   * permissions and limitations under the License.
>   *
> - * Copyright 2008 Rogue Wave Software.
> - * 
> + * Copyright 2008 Rogue Wave Software, Inc.
> + *
>   **************************************************************************/
>  
>  #ifndef _RWSTD_RW_REF_WRAP_INCLUDED
> @@ -36,17 +36,66 @@
>  #  if !defined _RWSTD_NO_EXT_CXX_0X
>  
>  
> -_RWSTD_NAMESPACE (__rw) {
> +_RWSTD_NAMESPACE (std) {
>  
>  
>  template <class _Type>
> -class __rw_ref_wrap
> +class reference_wrapper
>  {
> +    _Type* _C_ptr;
> +
> +public:
> +
> +    typedef _Type type;
> +
> +    reference_wrapper (_Type& __x)
> +        : _C_ptr (&__x) { /* empty */ }
> +
> +    reference_wrapper (const reference_wrapper<_Type>& __x)
> +        : _C_ptr (__x._C_ptr) { /* empty */ }
> +
> +    reference_wrapper& operator= (const reference_wrapper<_Type>& __x)
{
> +        _C_ptr = __x._C_ptr;
> +        return *this;

   1. We prefer to use the public types in favor of those of template
      parameters in definitions of standard templates. This is in
      contrast to the spec which prefers the template paramaters for
      some unknown reason. Our rationale is that the public names are
      more stable and more familiar to users and maintainers alike.

   2. We omit redundant template arguments in the definition of
      a template class. The rationale is simplicity.
      Thus, the declaration of the assignment operator should look
      like so:

      reference_wrapper& operator= (const reference_wrapper& __x)

   3. We omit definitions of special member functions (ctors, non
      virtual dtors, and assignment operators) that are normally
      implicitly generated by the compiler (provided the effects
      are right, of course). The rationale is efficiency and
      simplicity.
      In reference_wrapper, we can safely omit the definition of
      not just the dtor but also that of the copy ctor and the
      copy assignment operator.

> +    }
>  
> +    operator _Type& () const { return *_C_ptr; }
> +
> +    _Type& get() const { return *_C_ptr; }

   4. Function invariants should be asserted wherever possible.

   5. The definitions of even trivial non-empty functions should
      never appear on the same line as the function signature. I.e.,
      the above should be:

      type& get() const {
          _RWSTD_ASSERT (0 != _C_ptr);
          return *_C_ptr;
      }


Mime
View raw message