Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:104541
Message-ID: <1551472286.15396.15.camel@schlueters.de>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Date: Fri, 01 Mar 2019 21:31:26 +0100
In-Reply-To: <CAF+90c-0wfuvYZoGYLv82g8OepshCdHQ2XPYyGdSp6C3vg7wGg@mail.gmail.com>
References: 
	<CAF+90c9XDkT2k4jGy+z++5Z67ga=Z-DjR3wjMdLp7zPaFcX5AQ@mail.gmail.com>
	 <1551457459.2096.19.camel@schlueters.de>
	 <CAF+90c-0wfuvYZoGYLv82g8OepshCdHQ2XPYyGdSp6C3vg7wGg@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] Allow throwing from __toString()
From: johannes@schlueters.de (Johannes =?ISO-8859-1?Q?Schl=FCter?=)

On Fr, 2019-03-01 at 18:28 +0100, Nikita Popov wrote:
> made the same mistake there. So there's one more point to add to the
> extension guidelines:

Do we have a persistent space to keep those? The internals manual is
limited ... :-)

> I'm happy to find and fix these bugs, rather than leave the language
> buggy by design ;)

The "old man" in me is skeptical, especially since literally the second
program variation I tried broke in a really subtle way which is hard to
debug and understand for users.

Also I claim that if a string conversion throws an error, it's probably
no good idea to do that conversion in an implicit magic function. (but
yeah, that architectural choice doesn't necessarily have to be a
restriction in the language)


The third thing I tried was around sort(), where we don't have
exception guarantees specified and work on a reference:

    <?php
    $a = [
      'a',
      new throws,
      'b',
    ];

    try {
      sort($a);
    } catch (Exception $e) {
    }
    print_r($a);
    ?>

results in

    Array
    (
        [0] => a
        [1] => b
        [2] => throws Object
            (
            )

    )

Thus data is modified, while an exception is thrown. But I believe this
case isn't bad, while I assume there might be other cases, where the
effect has impact. (references should be avoided in idiomatic PHP
anyways ...)



Anyways, to help more productively with your young spirit:

Years back, when we changed string length handling, I created a clang-
analyzer plugin, which checks zend_parse_paramters() calls for the
right type.
https://github.com/johannes/clang-php-checker

I assume it should be possible to create a clang-analyz (or clang-tidy) 
check which ensures that conversions are done properly and are checking
for exceptions. This might help third-party extension authors (and if
integrated with the build also long-term ...) with such requirements.

johannes