Re: [patch] PR 8761 and 7076, take 2

[Nathan Myers <ncm-nospam at cantrip dot org>]

Some inane comments.

On Thu, Jan 16, 2003 at 01:08:53AM -0500, Jerry Quinn wrote:
>      // Guaranteed storage.
> +    // The first 5 iword and pword slots are reserved.  Currently
> +    // pword(0) is used for the formatting cache.
> +
>      static const int 	_S_local_word_size = 8;
>      _Words  		_M_local_word[_S_local_word_size];  

After all the discussion about reserved slots, it's embarrassing to
bring this up, but: The pword() mechanism works, but is a clumsy way 
to achieve the goal.

Given the right to break the ABI, the pointer stored in the pword()
slot might better be a regular member of ios_base.   The only reason
I made it a pword() slot, originally, was that the cache was meant
also as an example for user code which would benefit from using the 
same techniques.

Be very careful about copy semantics for ios_base -- if the pword()
element gets copied, is the cache shared?  Is such sharing MT-safe?
Or, have you registered a callback to make sure it doesn't get shared?
(I should read the code and see for myself.)

> +  // 1,222,444 == __grouping_tmp of "/1/3/3"
> +  // 1,222,444 == __grouping of "/3" == "/3/3/3"

Shouldn't the comments above say instead, e.g., "\3" == "\3\3\3" ?
(I know you copied them from the existing code, but let's fix
as we go.)

> +      // Forward decls and Friends:
> +      friend class locale;
> +      template<typename _Char, typename _InIter>
> +        friend class _Numeric_get;
> +      friend class num_get<_CharT>;
> +      friend class time_get<_CharT>;
> +      friend class money_get<_CharT>;
> +      friend class num_put<_CharT>;
> +      friend class time_put<_CharT>;
> +      friend class money_put<_CharT>;

Are these friend declarations really used?  I can't imagine how.

> +      // Given a member of the ios heirarchy as an argument, extract out all

s/heirarchy/hierarchy/

> +      // the current formatting information into a _Format_cache object and
> +      // return a pointer to it.  The __valid flag gives a way to force the
> +      // cache to be reconstructed from the current locale.
> +      static _Format_cache<_CharT>*
> +      _S_get(ios_base& __ios, bool __valid=true);

I am suspicious of this default argument in implementation apparatus.
Does it earn its keep?

> +  // num_put::_M_convert_int is only ever templatized on long, unsigned long, long
> +  // long, and unsigned long long.  

I don't know what "templatized" means in the context above.  Does that
mean "specialized", "explicitly specialized", or something else?

> +      if (__flags & ios_base::dec)
> +      else if (__flags & ios_base::oct)
> +      else // Hex

Is that the right algorithm?  I seem to recall that the standard
says __flags gets compared for equality to ios_base::dec and oct,
and that anything else is decimal.  Unfortunately that slows down
the common case.  The test might reasonably be moved to the code
that sets __flags, and sets a "decimal" flag in the cache for use
here.

> +  // As far as I can tell, this more general form will never be invoked.
> +  template<typename _CharT, typename _OutIter>
> +    inline
> +    _OutIter
> +    __num_put_output(_OutIter __s, const _CharT* __ws, int __len)

Best just leave it out.  You can add it back if it turns out to be useful.

Nathan Myers
ncm@cantrip.org