Return value of convert_to_*()

php.internals

Andrei Zmievski

19 years ago
Antony and I just had a spirited discussion on IRC about his latest patches to convert_to_unicode() and convert_to_string(). The specific troublesome point was conversion of IS_OBJECT type. His point was that since all we care about is the end result, then the function should return SUCCESS regardless of whether the object could convert itself to a string or whether we forced the conversion and got a simple "Object" string. I do not subscribe to this point of view. I added return values to conversion functions precisely because I wanted to distinguish between "good" and "forced" conversions. We can convert anything to anything, that's not a problem, but in certain cases I do care that we tried to convert IS_OBJECT to IS_STRING and couldn't do it without falling back onto the "Object" crutch. One such place is zend_parse_parameters(). If you say zpp(..., "s", &str, &str_len) and the user passes an object without __toString() method, zpp() should issue an error saying that it expected a string and got an object instead, even if we "forced" the conversion result to be a string in convert_to_string(). My problem with considering "Object" a "successful" conversion is this: why do we issue E_RECOVERABLE error if the object couldn't convert itself to a string? Why should the user be able to catch that and not my code? Please tell me that this is a valid point of view. -Andrei

Antony Dovgal

19 years ago
On 12/27/2006 11:02 PM, Andrei Zmievski wrote:
> Antony and I just had a spirited discussion on IRC about his latest > patches to convert_to_unicode() and convert_to_string(). The specific > troublesome point was conversion of IS_OBJECT type. His point was that > since all we care about is the end result, then the function should > return SUCCESS regardless of whether the object could convert itself to > a string or whether we forced the conversion and got a simple "Object" > string. I do not subscribe to this point of view. I added return values > to conversion functions precisely because I wanted to distinguish > between "good" and "forced" conversions.
Why do you want that? "Bad" conversion is failed conversion, that's the case when you get E_RECOVERABLE_ERROR and it bails out. I consider all the other cases are "good" conversions, since we've got the result we requested and have SUCCESSfully converted the object into string.
> One such place is zend_parse_parameters(). If you say zpp(..., "s", &str, &str_len) and > the user passes an object without __toString() method, zpp() should > issue an error saying that it expected a string and got an object > instead, even if we "forced" the conversion result to be a string in > convert_to_string().
An error? You mean this one: zend_error(E_NOTICE, "Object of class %v to string conversion", Z_OBJCE_P(op)->name); ? We already have it, no need for a new error. It's perfectly legal to do it in 5.2 and I don't see why PHP6 is different. IMO it's same as changing the E_RECOVERABLE to E_ERROR - users won't have any way to "workaround" it, even though this was the original intention of E_RECOVERABLE.
-- Wbr, Antony Dovgal

Andrei Zmievski

19 years ago
> Why do you want that? > "Bad" conversion is failed conversion, that's the case when you get > E_RECOVERABLE_ERROR and it bails out.
Yes, I want that function to return FAILURE when we issue E_RECOVERABLE_ERROR. Exactly what I was talking about.
> >> One such place is zend_parse_parameters(). If you say zpp(..., >> "s", &str, &str_len) and the user passes an object without >> __toString() method, zpp() should issue an error saying that it >> expected a string and got an object instead, even if we "forced" >> the conversion result to be a string in convert_to_string(). > > An error? You mean this one: > zend_error(E_NOTICE, "Object of class %v to string conversion", > Z_OBJCE_P(op)->name); > ? > We already have it, no need for a new error.
I want zend_parse_parameters() to issue a warning, saying "foo() expects parameter 1 to be string, object given". I do not consider "Object" to be a valid conversion for this purpose.
> It's perfectly legal to do it in 5.2 and I don't see why PHP6 is > different. > IMO it's same as changing the E_RECOVERABLE to E_ERROR - users > won't have any way to "workaround" it, even though this was the > original intention of E_RECOVERABLE.
It is not the same. E_ERROR stops execution. Returning FAILURE from convert_to_string() is simply a flag that lets calling code know what happened. -Andrei

Antony Dovgal

19 years ago
On 12/28/2006 12:12 AM, Andrei Zmievski wrote:
>> It's perfectly legal to do it in 5.2 and I don't see why PHP6 is >> different. >> IMO it's same as changing the E_RECOVERABLE to E_ERROR - users >> won't have any way to "workaround" it, even though this was the >> original intention of E_RECOVERABLE. > > It is not the same. E_ERROR stops execution. Returning FAILURE from > convert_to_string() is simply a flag that lets calling code know what > happened.
Failure in zend_parse_parameters() means that function itself will not be executed, which is a major change in behaviour and I don't think we really want it.
-- Wbr, Antony Dovgal

Andrei Zmievski

19 years ago
Only in cases where people blindly pass objects where strings are expected. It won't break anything for those objects that know how to convert themselves. -Andrei On Dec 27, 2006, at 1:23 PM, Antony Dovgal wrote:

Antony Dovgal

19 years ago
On 01/03/2007 09:51 PM, Andrei Zmievski wrote:
> Only in cases where people blindly pass objects where strings are > expected. It won't break anything for those objects that know how to > convert themselves.
It still see no reasons for this particular change in behavior. It was allowed in the past and I'm sure it should be still okay to do it as long as there are no technical reasons to prevent it, which I'm unable to see.
> On Dec 27, 2006, at 1:23 PM, Antony Dovgal wrote: > >> On 12/28/2006 12:12 AM, Andrei Zmievski wrote: >>>> It's perfectly legal to do it in 5.2 and I don't see why PHP6 is >>>> different. >>>> IMO it's same as changing the E_RECOVERABLE to E_ERROR - users >>>> won't have any way to "workaround" it, even though this was the >>>> original intention of E_RECOVERABLE. >>> It is not the same. E_ERROR stops execution. Returning FAILURE from >>> convert_to_string() is simply a flag that lets calling code know what >>> happened. >> >> Failure in zend_parse_parameters() means that function itself will not >> be executed, which is a major change in behaviour and I don't think we >> really want it. >> >> -- >> Wbr, Antony Dovgal >
-- Wbr, Antony Dovgal

Andrei Zmievski

19 years ago
So you're okay with substr($obj, 0, 3) giving you "Obj" ?? To me, that seems broken, regardless of whether we had it "working" in the past. -Andrei On Jan 3, 2007, at 10:58 AM, Antony Dovgal wrote:

Marcus Börger

19 years ago
Hello Andrei, we had a long decision about this issue already and decided that the only reason for "Object..." in 5.0 and 5.1 was the lack of ability to fix __toString() prior to 5.2. The current 5.2/6.0 behavior is what we wanted in the first place and hence correct, or do we need to restart the discussion again? Or am I again missing something here? best regards marcus Wednesday, January 3, 2007, 8:17:53 PM, you wrote:
> So you're okay with substr($obj, 0, 3) giving you "Obj" ??
> To me, that seems broken, regardless of whether we had it "working" in > the past.
> -Andrei
> On Jan 3, 2007, at 10:58 AM, Antony Dovgal wrote:
>> On 01/03/2007 09:51 PM, Andrei Zmievski wrote: >>> Only in cases where people blindly pass objects where strings are >>> expected. It won't break anything for those objects that know how to >>> convert themselves. >> >> It still see no reasons for this particular change in behavior. >> It was allowed in the past and I'm sure it should be still okay to do >> it as long as there are no technical reasons to prevent it, which I'm >> unable to see. >> >> >>> On Dec 27, 2006, at 1:23 PM, Antony Dovgal wrote: >>>> On 12/28/2006 12:12 AM, Andrei Zmievski wrote: >>>>>> It's perfectly legal to do it in 5.2 and I don't see why PHP6 is >>>>>> different. >>>>>> IMO it's same as changing the E_RECOVERABLE to E_ERROR - users >>>>>> won't have any way to "workaround" it, even though this was the >>>>>> original intention of E_RECOVERABLE. >>>>> It is not the same. E_ERROR stops execution. Returning FAILURE from >>>>> convert_to_string() is simply a flag that lets calling code know >>>>> what happened. >>>> >>>> Failure in zend_parse_parameters() means that function itself will >>>> not be executed, which is a major change in behaviour and I don't >>>> think we really want it. >>>> >>>> -- >>>> Wbr, Antony Dovgal >> >> >> -- >> Wbr, Antony Dovgal
Best regards, Marcus

Andrei Zmievski

19 years ago
On Jan 3, 2007, at 12:00 PM, Marcus Boerger wrote:
> Hello Andrei, > > we had a long decision about this issue already and decided that the > only reason for "Object..." in 5.0 and 5.1 was the lack of ability to > fix __toString() prior to 5.2. The current 5.2/6.0 behavior is what we > wanted in the first place and hence correct, or do we need to restart > the discussion again? Or am I again missing something here?
I guess it's fine, since it's a catchable fatal error. -Andrei

Antony Dovgal

19 years ago
On 01/03/2007 10:17 PM, Andrei Zmievski wrote:
> So you're okay with substr($obj, 0, 3) giving you "Obj" ?? > > To me, that seems broken, regardless of whether we had it "working" in > the past.
I can name you quite a number of things which seem broken to me (or aesthetically wrong), but I don't think it means I can "fix" them freely..
-- Wbr, Antony Dovgal