We should note that std::string does NOT have a 'operator char *', and perhaps 
cautiously ponder why that might be. (Neither does std::string have 'operator 
bool', that's probably also worth considering.)

I'm not sure why 'operator char *' makes me more nervous than 'operator 
std::string'.

I also just realized, ustring::str() returns a std::string&, so if you pass 
myustring.str() to something that wants a const std::string&, it won't create a 
new string copy. It's not clear to me yet wether we can make 'operator const 
std::string&', which is really more like what we want. I have a feeling there 
is no way to make that happen.

Hmmm, several things to consider. This is one to consider very carefully before 
adding or subtracting any casts. That feels more dangerous than ordinary 
methods that you'd have to call explicitly.



On Jan 26, 2015, at 9:11 AM, John Haddon <[email protected]> wrote:

> As someone who merrily plunged headfirst into this trap a little while back, 
> I thought I might chip in. I don't recall the exact specifics, but I think my 
> problem was triggered when OSLQuery changed a bunch of members from string to 
> ustring a little while back. I had existing code that passed those members to 
> methods expecting std::string, and the code compiled fine with the ustring 
> change, but crashed at runtime. I think the compiler was casting ustring to 
> (invalid) char * via this conversion, and then constructing std::string from 
> invalid memory - does that sound plausible?. It took a while to figure out, 
> because my assumption was "ustring behaves like string as far as possible".
> 
> In my case I think a specific cast operator to std::string would have kept 
> everything working as intended without change, so that seems like a pretty 
> reasonable approach at least for the specific issue I encountered…
> 
> Cheers…
> John
> 
> 
> From: Oiio-dev [[email protected]] on behalf of Thiago 
> Ize [[email protected]]
> Sent: Monday, January 26, 2015 8:40 AM
> To: OpenImageIO developers
> Subject: Re: [Oiio-dev] ustring::operator int() is bad idea?
> 
> An operator std::string() function would be great if you can get that to 
> work!  Of course, anything else that was expecting some kind of integer, such 
> as a char or int, will still get caught in this, but until OIIO switches to 
> c++11, I guess this is good enough at getting at least this common case 
> covered.
> 
> Speaking of operator std::string(), if we're going to do that, would it be a 
> good idea to also add an operator const char*()?  Sadly, that won't work with 
> printf since that takes in a vararg, but it would work for regular functions 
> that take in a const char*, such as atoi().  Adding that also has the nice 
> benefit of making the original issue I reported ambiguous. 
> 
> Suppose we have a custom string class called mystring which just like 
> std::string will allow a mystring to be created from either a char or a 
> char*.  Then:
> 
> my_string("string B");
> my_string = my_ustring;
> 
> will report:
> foo.cpp:13:14: error: use of overloaded operator '=' is ambiguous (with 
> operand types 'mystring' and
>       'OpenImageIO::v1_4::ustring')
>    my_string = my_ustring;
>    ~~~~~~~~~ ^ ~~~~~~~~~~
> foo.h:507:7: note: 
>       candidate function
>       operator=(const _char* __s) 
>       ^
> foo.h:518:7: note: 
>       candidate function
>       operator=(char __c) 
>        ^
> 
> This is actually a nice error since it at least prevents this bug from 
> slipping through in non std::string classes and we thereby have all our major 
> use cases covered.  Yes, I guess if someone tried to assign a ustring 
> directly to a char then they won't get an error.  In that case, maybe 
> rewriting operator int() to be:
> 
> operator int (void) const {
>    if (!m_chars) return 0;
>    return m_chars[0];
> }
> 
> or perhaps better yet, replacing it with a char operator
> 
> operator char (void) const {
>    if (!m_chars) return 0;
>    return m_chars[0];
> }
> 
> would be an even better solution since now we can have all our bases covered. 
>  If someone tries to write directly to a char:
> char c = ustring("x");
> it will work in a sane truncating kind of way.  if(my_ustring) will still 
> work just like before. And if someone writes directly to a const char*, the 
> operator const char* will catch it, as will the operator std::string for 
> std::string.  And for anything else that is ambiguous, the compiler will 
> complain. 
> 
> On Sun, Jan 25, 2015 at 11:52 PM, Larry Gritz <[email protected]> wrote:
> I totally see what you're getting at and agree that it's a potential problem. 
> (Though the real source of the problem is that in C++ is that 0 casts just 
> tine to a pointer, instead of having a proper pointer type, like nullptr is 
> in C++11. I digress, that observation doesn't actually help us solve this 
> problem in practice.)
> 
> I'm very afraid of breaking existing code, and also I am still rather fond of 
> the "if (s)" idiom.
> 
> Might another way of dealing with this be to add
> 
>         operator std::string() const { return this->string(); }
> 
> That would allow assignment of ustring to std::string to do the obvious thing.
> 
> Would that suffice, or can you think of other scenarios in which operator 
> bool is likely to be an ongoing problem?
> 
>         -- lg
> 
> 
> 
> On Jan 25, 2015, at 6:24 PM, Thiago Ize <[email protected]> wrote:
> 
> > I'm wondering if the following ustring function:
> >
> >     /// Cast to int, which is interpreted as testing whether it's not an
> >     /// empty string.  This allows you to write "if (t)" with the same
> >     /// semantics as if it were a char*.
> >     operator int (void) const { return !empty(); }
> >
> > is a good idea?  Reason being that this allows for what is probably 
> > unexpected behavior in the common use case of trying to assign a ustring to 
> > a std::string (note how this case could also happen from a simple typo if 
> > the "u" in ustring was accidentally omitted).  For instance:
> >    ustring my_ustring("string A");
> >    string my_string("string B");
> >    my_string = my_ustring;
> >    printf(" my_string=\"%s\" and\n my_ustring=\"%s\"\n", my_string.c_str(), 
> > my_ustring.c_str());
> >
> > will print out:
> >  my_string="" and
> >  my_ustring="string A"
> >
> > which is most likely not the intended result.  If we get rid of this 
> > function, then the compiler will point out our mistake:
> > foo.cpp:13:14: error: no viable overloaded '='
> >    my_string = my_ustring;
> >
> > I realize getting rid of this function would break lots of preexisting 
> > code, but I think this might be worth it in order to prevent this type of 
> > problem.  If it's too big of a change, then at least it might be worth 
> > adding a comment to this function indicating that it will cause this 
> > problem.
> >
> 
> --
> Larry Gritz
> [email protected]
> 
> 
> 
> _______________________________________________
> Oiio-dev mailing list
> [email protected]
> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
> 
> _______________________________________________
> Oiio-dev mailing list
> [email protected]
> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org

--
Larry Gritz
[email protected]



_______________________________________________
Oiio-dev mailing list
[email protected]
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org

Reply via email to