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]<mailto:[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]<mailto:[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]<mailto:[email protected]> _______________________________________________ Oiio-dev mailing list [email protected]<mailto:[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
