Re: [boost] Re: lexical_cast<>() and character traits

From: Terje Slettebø (tslettebo_at_[hidden])
Date: 2002-07-24 23:28:15

>From: "Gennaro Prota" <gennaro_prota_at_[hidden]>

>Seriously, I've looked at the new version and I find it very nice.

Thanks. :)

>Only two points:

>1) About pointer_to_char_base: maybe someone will argue whether
>something like

>template<class Target, class Source>
>    struct pointer_to_char_base
>    {
>      static Target do_cast(Source arg)
>      {
>        return arg[0];
>      }
>    };

>isn't preferable.

Let's see, the current version is:

    template<class Target, class Source>
    struct pointer_to_char_base
    {
      static Target do_cast(Source arg)
      {
        if(arg[1]!=NULL)
          throw bad_lexical_cast();

        return arg[0];
      }
    };

By the way, I wasn't actually sure what to put as string-terminator, there.
'\0' might be an alternative, but as this is supposed to work for other
character types, as well (such as wchar_t), as I understand, this would mean
that for wchar_t, you would have to have L'\0', instead. Therefore, I
guessed that NULL or 0 would be ok..

I've tested for 0/NULL there, to ensure that the character string is of
length 1 (although I see it doesn't test for empty string, as you mention
below, here). Otherwise, it would not fit in a char, or the char would get
the null-terminator, in the case of an empty string. The above base class
handles the cases of "(const) char_type *" -> "char_type". Without the test,
it would allow a string of any length, or empty string, to be "converted" to
a char (resulting in the first character of the string, if any), which is
clearly wrong.

lexical_cast is supposed to throw a bad_lexical_cast, if the conversion
can't be done, and this case, I think that would be appropriate, as one
character isn't a full representation of a several character string, or
empty string.

Comments?

>In any case, the current implementation triggers undefined behavior
>with empty source strings; a trivial fix is:

>    template<class Target, class Source>
>    struct pointer_to_char_base
>    {
>      static Target do_cast(Source arg)
>      {
>        Target t = arg[0];
>        if(t != 0  &&  arg[1] != 0)
>          throw bad_lexical_cast();
>
>        return t;
>      }
>    };

Opps, you're right. Will be fixed. This is one that was missed by the unit
tests, simply because there was no test to test a case where it should throw
an exception. The test system is made to enable such tests, as well, I just
hadn't included those tests. I'll add tests that check that it throws an
exception when expected, as well.

Isn't it typical? The one thing you don't test for, is where you get a bug.
:)

Actually, the code above doesn't throw an exception, if you try to convert
an empty string to a char, which I think it should, as mentioned, so it
should probably be changed to:

if(arg[0] == 0 || arg[1] != 0)
  throw bad_lexical_cast();

This is now fixed, and unit tests to check that exceptions are thrown when
expected, are added, and the new version is uploaded
(http://groups.yahoo.com/group/boost/files/lexical_cast_proposition/).

>2) Stupid question of the day: is there any reason why all Source
>function parameters cannot be declared as Source const &?

It's not a stupid question. :) Well, for some types, it may be more
efficient to pass by value, than by reference. Pass by reference typically
passes the address of the object, so for small types, just passing the
object may be more efficient, as you then avoid the indirection, when
operating on the object. Therefore, boost::call_traits (which does exactly
this selection between pass by value, and pass by const reference, based on
the size of the object) is used in the general case (base template), while
the specialisations use what is appropriate there, generally like
call_traits would do it.

>P.S.: Does anyone know if Intel C++ 6.0 for Windows can properly
>support wchar_t as a distinct type? I've also experimented with the
>new /Zc:wchar_t switch. Well, if you try the following:
>
>#include <iostream>
>
>void dummy (unsigned short par) {
>
>    std::cout << "wrong overload... - ";
>    std::cout << par;
>
>}
>
>void dummy (wchar_t par) {
>    std::wcout << L"wchar_t! - ";
>    std::wcout << par;
>}
>
>int main() {
>  dummy (L'1');
>}
>
>
>it (correctly) compiles only with the switch, but (incorrectly) prints
>"wrong overload... - 49"! :-(

That was odd, I get the same. I use Intel C++ 6.0 for Windows, myself, and
I've also found the /Zc:wchar_t switch in the docs, as it otherwise emulates
MSVC 6.0, which has no wchar_t intrinsic type.

However, it appears the support may not be complete. Perhaps report this
Intel? If not, I could do it.

By the way, this works correctly on Intel C++ 7.0 pre-beta... Perhaps if you
complain about this, you'll get that, too. :) I got that version, after I
reported some ICE when trying to compile BLL. They gave me that, to try
again. It still doesn't work, but other things, such as this, does.

There are also other things that work on 7.0 pre-beta, such as Loki's
SmartPtr.h, which doesn't work on 6.0.

Thanks for the feedback.

Regards,

Terje