Fix GH-8329 Print true/false instead of bool in error and debug messages

[kocsismate]

As false has recently became a type on its own and true is is going to be one soon, we can now print false/true in error and debug messages instead of just bool.

[nikic]

Not fully convinced of this change. Some of these error messages makes sense, like

property_exists(): Argument #1 ($object_or_class) must be of type object|string, true given

while others don't, like

Warning: Trying to access array offset on value of type true

For error messages that explicitly mention "type" using true and false isn't really appropriate.

[kocsismate]

while others don't, like
Warning: Trying to access array offset on value of type true

Yeah, I agree. For this very example, I didn't notice that the new wording didn't make sense with the old message anymore, so I'll fix it up.

For other messages where displaying true/false doesn't make sense (like the one I highlighted in a comment), my idea is to either revert the change, or to wait until true becomes a type. :)

[mvorisek]

https://3v4l.org/jvaCB true type is now native type, can this PR be merged?

[Girgias]

https://3v4l.org/jvaCB true type is now native type, can this PR be merged?

True might be a native type but it remains a literal type i.e. a value within a type. So I'm sceptical of the improvement.

[mvorisek]

I opened the #8329 issue to be able to better understand error messages, very often when I see Argument #1 ($obj) must be of type object, bool given the problem is false was given, and displaying the exact bool value provides the user higher value than just the type.

This is also the case for object type - https://3v4l.org/XOZBB - specific passed class is displayed than just object.

[mvorisek]

@cmb69 @bwoebi what is your opinion on this?

[bwoebi]

Sounds good to me - I see not reason to not be more specific when trivially possible (and a mere boolean is not leaking sensitive info on its own).

[kocsismate]

I've just pushed an update to the PR where:

• I introduced the zend_zval_value_name() function which returns true/false unlike to zend_zval_type_name() returning bool
• I used the new function everywhere where a the message is related to a value (usually an argument)
• I left the original message intact where the message is related to a type. One notable exception is the Trying to access array offset on value of type bool message where I simplified the message and now it's like Trying to access array offset on false

[nikic]

Looks reasonable from a quick glance. Do you have an example where zend_zval_type_name is still used? (Hard to see absence of changes in a diff...)

[kocsismate]

Looks reasonable from a quick glance. Do you have an example where zend_zval_type_name is still used? (Hard to see absence of changes in a diff...)

Mostly it's when we use "Value of type ..." in the message: Value of type %s returned from %s::__get() must be compatible with unset property %s::$%s of type %s, Value of type %s is not callable, Argument of type %s will be interpreted as string in the future.

Two other examples:

• When ReflectionExtension::__toString() returns the type of constants
• Unsupported operand types: %s %s %s

I guess some of these messages could also be changed in a way which suits the new literal types... but I'm not interested in doing more improvements in this area, so I'd let the possibility to someone else, unless there is a straightforward and consistent way to fix them.

[cmb69]

The test expectations of ext/opcache/tests/jit/fetch_dim_func_arg_002.phpt and ext/opcache/tests/opt/assign_op_001.phpt need to be adjusted.

[cmb69]

• I used the new function everywhere where a the message is related to a value (usually an argument)
• I left the original message intact where the message is related to a type. […]

I like that. Thank you!