Hey Internals,

So, I received a message that said of the mail I should have received from 
internals bounced; so, this is part test and part update.

I’ve started mapping out call paths in the project that brought this concept to 
light.

The project, Shoop, never uses is_bool() - instead it always uses empty, then 
reverses that for:

1. Booleans
2. Numbers
3. Arrays
4. Strings
5. stdClass or data-only classes
6. Json strings, which it treats as #5

I essentially get standard SPL behavior when I do this.

For the object definition I can define two interfaces:

1. Falsifiable
2. Emptiable

The checks go like this:

1. if the object implements Falsifiable: the __toBool method would be called 
and would return the result. (This check would be inside is_bool() and could 
use the empty() implementation)
2. if the object implements Emptiable: the __isempty method would be called and 
return the result. (This check would be inside empty())
3. else: standard output from is_bool() or empty() would be used when passing 
an instance to either of those SPL functions, depending on which the user is 
using.

Because the concepts of emptiness and falsiness are so tightly coupled, I’m 
wondering if it would be better to implement both with this RFC??

Otherwise, Emptiable would be a future enhancement consideration.

I’d like to hear what the rest of Internals thinks.

Next for me: I’m going to finish solidifying the Shoop project and make sure my 
other projects can use latest and then continue going through the tutorials 
from Nikita and others on doing development Internals.

Cheers,
Josh

> On Aug 30, 2020, at 9:32 AM, Josh Bruce <j...@joshbruce.dev> wrote:
> 
> Hey Tyson,
> 
> This is great! Thank you so much, sincerely.
> 
> Still slow goings, which is fine, we have at least a year. lol
> 
> Static analyzers seem to be the biggest concern to date.
> 
> Haven’t been able to get one running locally - though I’ve only spent a few 
> minutes here and there; definitely on the list.
> 
> A use case for this sort of thing is also a concern or question. I came 
> across Pipeline from the PHP League and saw their interruptible processor and 
> was wondering if something like this would be helpful there - for pipeline 
> patterns in general: 
> https://github.com/thephpleague/pipeline/blob/master/src/InterruptibleProcessor.php
>  
> <https://github.com/thephpleague/pipeline/blob/master/src/InterruptibleProcessor.php>
> 
> While working on another project, saw this line from the PHP array_filter 
> docs:
> 
>     "If no callback is supplied, all entries of array equal to FALSE (see 
> converting to boolean) will be removed."
> 
> I’m still field testing the latest iteration of my base project, but wanted 
> to put a working (non-internals) implementation out there (note this covers 
> emptiness and falseness for the purposes of the project):
> 
> Tests - 
> https://github.com/8fold/php-shoop/blob/master/tests/RfcObjectCanBeDeclaredFalsifiableTest.php
>  
> <https://github.com/8fold/php-shoop/blob/master/tests/RfcObjectCanBeDeclaredFalsifiableTest.php>
> 
> Interface - 
> https://github.com/8fold/php-shoop/blob/master/src/FilterContracts/Interfaces/Falsifiable.php
>  
> <https://github.com/8fold/php-shoop/blob/master/src/FilterContracts/Interfaces/Falsifiable.php>
>  - for our purposes the efToBool would be __toBool
> 
> Default implementation - 
> https://github.com/8fold/php-shoop/blob/master/src/Shooped.php#L216 
> <https://github.com/8fold/php-shoop/blob/master/src/Shooped.php#L216>
> 
> “Type system” implementation - 
> https://github.com/8fold/php-shoop/blob/master/src/Filter/TypeAsBoolean.php 
> <https://github.com/8fold/php-shoop/blob/master/src/Filter/TypeAsBoolean.php>
> 
> Cheers,
> Josh
> 
>> On Aug 9, 2020, at 3:59 PM, tyson andre <tysonandre...@hotmail.com 
>> <mailto:tysonandre...@hotmail.com>> wrote:
>> 
>> Hi Josh,
>> 
>> I'd recommend looking at https://github.com/php/php-src/pull/5156 
>> <https://github.com/php/php-src/pull/5156> (linked to in your RFC's 
>> reference) when implementing this PR.
>> Additionally, I didn't see if this was brought up - PHP already has a class 
>> which is falsifiable - SimpleXMLElement.
>> 
>> You can see the source of ext/simplexml/simplexml.c for `_IS_BOOL`
>> 
>> ```
>> static int sxe_object_cast_ex(zend_object *readobj, zval *writeobj, int type)
>> {
>>      php_sxe_object *sxe;
>>      xmlChar        *contents = NULL;
>>      xmlNodePtr          node;
>>      int rv;
>> 
>>      sxe = php_sxe_fetch_object(readobj);
>> 
>>      if (type == _IS_BOOL) {
>>              node = php_sxe_get_first_node(sxe, NULL);
>>              if (node) {
>>                      ZVAL_TRUE(writeobj);
>>              } else {
>>                      ZVAL_BOOL(writeobj, !sxe_prop_is_empty(readobj));
>>              }
>>              return SUCCESS;
>>      }
>> ```
>> 
>> ```
>> static php_sxe_object* php_sxe_object_new(zend_class_entry *ce, 
>> zend_function *fptr_count)
>>      intern->zo.handlers = &sxe_object_handlers;
>> // ... elsewhere in ext/simplexml/simplexml.c
>>      sxe_object_handlers.cast_object = sxe_object_cast;
>> ```
>> 
>> The default handlers would be overridden, so this would probably need to 
>> fall back to the original handlers for types other than bool.
>> Also, this might need to copy all of the handlers from the parent class 
>> entry's zend_object_handlers,
>> to meet the expected behavior of other cast types and other magic behaviors 
>> from SimpleXMLElement and other classes continuing to work.
>> 
>> From the perspective of static analysis, a few things to note:
>> - Static analyzers such as psalm/phan currently deliberately don't bother 
>> handling the possibility that `object` can be false even after it was 
>> checked for falsiness.
>>  There's hundreds of other things that could be implemented, and adding the 
>> special casing and performance overhead checking for FFI objects and 
>> SimpleXMLElement, subclasses of those is currently low priority compared to 
>> those things.
>> 
>>  Code using SimpleXMLElement/FFI is generally limited to a few files in 
>> practice.
>> 
>>  Definitely possible for analyzers to support this for known base classes, 
>> though, and the priority would increase if the RFC passed.
>> 
>>  I am a maintainer of Phan.
>> - For BC reasons, internal data structures such as ArrayObject probably 
>> wouldn't get changed in any php release
>>   (e.g. could break code using falsiness check instead of null check).
>>   So this might lead to inconsistencies with newer extensions if half of the 
>> data structures treat emptiness as false and half don't.
>> - This feature may end up getting adopted in cases where it's convenient but 
>> turns out prone to accidental bugs and later is deprecated and removed.
>>  For example, 
>> https://stackoverflow.com/questions/25031236/if-elem-vs-elem-is-not-none 
>> <https://stackoverflow.com/questions/25031236/if-elem-vs-elem-is-not-none> 
>> was seen in python
>> 
>>  (e.g. `if (!$this->resultSetObject) { $this->resultSetObject = 
>> slow_db_operation(); }`
>>  would not behave the way people would previously expect for most objects 
>> (executed repeatedly instead of once))
>> 
>> ```
>> <?php
>> function test(SimpleXMLElement $e) {
>>    // False positive RedundantCondition in psalm
>>    if ($e) {
>>    }
>> }
>> ``` 
>> 
>>> And I don’t know of a way to add an interface to new stdClass() - but 
>>> thought this might be a valid use case:
>> 
>> `stdClass` isn't a final class. I assume they meant this
>> 
>> ```
>> class EmptyStdClass extends stdClass implements Falsifiable {
>>    public function __toBool() { return false; }
>> }
>> function example(stdClass $s) {
>>    if (!$s) { throw new Exception("Previously impossible"); }
>> }
>> ```
>> 
>> Thanks,
>> - Tyson
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: https://www.php.net/unsub.php 
>> <https://www.php.net/unsub.php>
>> 
> 

Reply via email to