Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

[G. P. B.]

I am going to group the response from the 3 emails.

On Tue, 27 Jun 2023 at 09:17, Claude Pache  wrote:

> Although the specific comment in the php.ini file talks about runtime
> switching, it is in fact irrelevant whether it is set at runtime or not.
> More concretely; I use currently the settings:
>
> * `zend.assertion=1` as set in the global php.ini;
> * `assert.active = on` or `off` in my per-directory .user.ini or .htaccess
> file, depending on whether I want to enable assertions by default;
> * optionally  `ini_set('assert.active', '1')` at runtime when I want to
> enable debug mode temporarily.
>
> I have established the above settings several years ago (at the time of
> PHP 7), after having carefully read the documentation (and having been
> slightly confused by the redundancy between `assert.active` and
> `zend.assertion`). I might have end up to switch between `zend.assertion =
> 0/1` instead of switching between `assert.active = on/off` instead; I can’t
> say exactly why I chose the former option instead of the latter, but I
> guess that both the comment in the `php.ini` file, and the existence of
> `assert_options(ASSERT_ACTIVE, ...)` as an alternative to
> `ini_set('assert.active', ...)`, have influenced my choice.
>
> Note also that the above settings minus `zend.assertion` was the way to do
> it in PHP 5 (again it is irrelevant whether it is at runtime or not), and
> many people that used assertions in PHP 5 may have continued to use that
> way without modification, as there is currently no strong reason to change
> (only `zend.assertions=-1` is relevant if you are especially concerned
> about micro-optimisation).
>
> Now, of course, people will adapt to the new settings. But given the
> confusing state of the options (both `zend.assertion` and `assert.active`
> must be enabled, and I am not even speaking about `assert.exception`), you
> ought to be super-clear (in the deprecation notice, in the php.ini file, in
> the docs), what is the the expected state: paradoxically leave
> `assert.active` and `assert.exceptions` enabled (the default value) even
> when you want to disable assertions, and playing exclusively with
> `zend.assertion`.
>

I would argue that zend.assertions=-1 is more than a micro-optimisation,
but that not the point.
Yes, you would need to switch to toggle zend.assertions between 1 and 0 if
you would want to enable/disable assertions at run-time.
However, considering that code should behave the same even when assertions
are not compiled (with the -1 value) I don't really see the point of
toggling between both modes.

The expected state is to leave all the assert.* INI settings to their
default value and only use the zend.assertions INI setting.

The state of the assert() docs were in very bad shape, and I hope that my
recent changes to it has improved it and made them clearer.
However, I will make sure that the path forward is very clear be that in
the INI setting docs, the assert() docs, the assert_options() docs, the
migratrion guide, and in the deprecation message.


On Tue, 27 Jun 2023 at 09:37, Claude Pache  wrote:

> I don’t see the RFC listed under https://wiki.php.net/rfc#under_discussion
> .
>

Fixed.


> The RFC is imprecise in what is meant by “deprecating”. I guess that a
> deprecation notice (E_DEPRECATED) will be triggered at least under the
> following conditions:
>
> * at startup when one of the assert.* setting has not the default value;
> * at runtime when `assert_options(...)` is used;
> * at runtime when `ini_set(...)` is used to set an `assert.*` option to a
> non-default value?
>
> It is unclear to me what will happen when:
>
> * `ini_set(...)` is used to set an `assert.*` option to its default value,
> either as a no-op or not?
>

Clarified.


> Moreover, by removing `assert.callback` (and other options), are you
> effectively removing a feature? (I don’t really know, I have never
> considered it even in my worst dreams.) If so, it should be noted in a
> “Backward Incompatible Changes”.
>

Yes, I don't really see the point of such a section as a deprecation, and
thus removal is clearly a BC Break.
I have tried to improve the wording to convey this more clearly.

On Tue, 27 Jun 2023 at 09:47, Claude Pache  wrote:

> changing the return value of `assert(...)` from `bool` (true) to `void` is
> also a BC break, (and it is unclear to me what is the effective advantage
> of the break).
>

Considering that any failed assertion is going to abort execution, having
it return a value is pointless.
Moreover, one must not rely on the expression being asserted to be executed
(thus it should have no side effects) and changing it to void very clearly
conveys this meaning that one must not store the result of the assert() as
it is not a totally normal function call.

Best regards,

George P. Banyard

Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

[Claude Pache]

> Le 27 juin 2023 à 10:36, Claude Pache  a écrit :
> 
> 
> 
>> Le 26 juin 2023 à 17:06, G. P. B.  a écrit :
>> 
>> On Wed, 31 May 2023 at 13:08, G. P. B.  wrote:
>> 
>>> Hello internals,
>>> 
>>> I would like to start the discussion about deprecating various remains
>>> from the now removed string code evaluated assertions functionality of
>>> assert().
>>> 
>>> The RFC is located on the wiki at the following address:
>>> https://wiki.php.net/rfc/assert-string-eval-cleanup
>>> 
>>> Initially, this was part of the mass PHP 8.3 deprecation RFC, but only the
>>> assert_options() function was part of it.
>>> 
>> 
>> Head's up, I'm planning on opening the vote on this on Wednesday the 28th
>> of June.
>> 
>> Best regards,
>> 
>> George P. Banyard
> 
> Hi,
> 
> Still some points:
> 
> I don’t see the RFC listed under https://wiki.php.net/rfc#under_discussion.
> 
> The RFC is imprecise in what is meant by “deprecating”. I guess that a 
> deprecation notice (E_DEPRECATED) will be triggered at least under the 
> following conditions:
> 
> * at startup when one of the assert.* setting has not the default value;
> * at runtime when `assert_options(...)` is used;
> * at runtime when `ini_set(...)` is used to set an `assert.*` option to a 
> non-default value?
> 
> It is unclear to me what will happen when:
> 
> * `ini_set(...)` is used to set an `assert.*` option to its default value, 
> either as a no-op or not?
> 
> Moreover, by removing `assert.callback` (and other options), are you 
> effectively removing a feature? (I don’t really know, I have never considered 
> it even in my worst dreams.) If so, it should be noted in a “Backward 
> Incompatible Changes”.
> 
> —Claude
> 

... and, of course, changing the return value of `assert(...)` from `bool` 
(true) to `void` is also a BC break, (and it is unclear to me what is the 
effective advantage of the break).

—Claude

Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

[Claude Pache]

> Le 26 juin 2023 à 17:06, G. P. B.  a écrit :
> 
> On Wed, 31 May 2023 at 13:08, G. P. B.  wrote:
> 
>> Hello internals,
>> 
>> I would like to start the discussion about deprecating various remains
>> from the now removed string code evaluated assertions functionality of
>> assert().
>> 
>> The RFC is located on the wiki at the following address:
>> https://wiki.php.net/rfc/assert-string-eval-cleanup
>> 
>> Initially, this was part of the mass PHP 8.3 deprecation RFC, but only the
>> assert_options() function was part of it.
>> 
> 
> Head's up, I'm planning on opening the vote on this on Wednesday the 28th
> of June.
> 
> Best regards,
> 
> George P. Banyard

Hi,

Still some points:

I don’t see the RFC listed under https://wiki.php.net/rfc#under_discussion.

The RFC is imprecise in what is meant by “deprecating”. I guess that a 
deprecation notice (E_DEPRECATED) will be triggered at least under the 
following conditions:

* at startup when one of the assert.* setting has not the default value;
* at runtime when `assert_options(...)` is used;
* at runtime when `ini_set(...)` is used to set an `assert.*` option to a 
non-default value?

It is unclear to me what will happen when:

* `ini_set(...)` is used to set an `assert.*` option to its default value, 
either as a no-op or not?

Moreover, by removing `assert.callback` (and other options), are you 
effectively removing a feature? (I don’t really know, I have never considered 
it even in my worst dreams.) If so, it should be noted in a “Backward 
Incompatible Changes”.

—Claude

Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

[Claude Pache]

> Le 26 juin 2023 à 17:05, G. P. B.  a écrit :
> 
> On Wed, 31 May 2023 at 23:20, Claude Pache  > wrote:
>> Although your RFC says that the `zend.assertions` ini setting has superseded 
>> `assert.active` for a while, the “official” php.ini file still advises to 
>> modify the value of `assert.active` rather than the one of `zend.assertion` 
>> in order to switch behaviour at runtime:
>> 
>> https://github.com/php/php-src/blob/91fd5641cde138b8894a48c921929b6e4abd5c97/php.ini-development#L1604
>> 
>> I bet that many people (myself included) follows the advice given in 
>> `php.ini`.
> 
> This talks about run-time modification, which is something that I don't think 
> should be done in practice, nor is often done.

Although the specific comment in the php.ini file talks about runtime 
switching, it is in fact irrelevant whether it is set at runtime or not. More 
concretely; I use currently the settings:

* `zend.assertion=1` as set in the global php.ini;
* `assert.active = on` or `off` in my per-directory .user.ini or .htaccess 
file, depending on whether I want to enable assertions by default;
* optionally  `ini_set('assert.active', '1')` at runtime when I want to enable 
debug mode temporarily.

I have established the above settings several years ago (at the time of PHP 7), 
after having carefully read the documentation (and having been slightly 
confused by the redundancy between `assert.active` and `zend.assertion`). I 
might have end up to switch between `zend.assertion = 0/1` instead of switching 
between `assert.active = on/off` instead; I can’t say exactly why I chose the 
former option instead of the latter, but I guess that both the comment in the 
`php.ini` file, and the existence of `assert_options(ASSERT_ACTIVE, ...)` as an 
alternative to `ini_set('assert.active', ...)`, have influenced my choice.

Note also that the above settings minus `zend.assertion` was the way to do it 
in PHP 5 (again it is irrelevant whether it is at runtime or not), and many 
people that used assertions in PHP 5 may have continued to use that way without 
modification, as there is currently no strong reason to change (only 
`zend.assertions=-1` is relevant if you are especially concerned about 
micro-optimisation).

Now, of course, people will adapt to the new settings. But given the confusing 
state of the options (both `zend.assertion` and `assert.active` must be 
enabled, and I am not even speaking about `assert.exception`), you ought to be 
super-clear (in the deprecation notice, in the php.ini file, in the docs), what 
is the the expected state: paradoxically leave `assert.active` and 
`assert.exceptions` enabled (the default value) even when you want to disable 
assertions, and playing exclusively with `zend.assertion`.

—Claude

Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

[G. P. B.]

On Wed, 31 May 2023 at 13:08, G. P. B.  wrote:

> Hello internals,
>
> I would like to start the discussion about deprecating various remains
> from the now removed string code evaluated assertions functionality of
> assert().
>
> The RFC is located on the wiki at the following address:
> https://wiki.php.net/rfc/assert-string-eval-cleanup
>
> Initially, this was part of the mass PHP 8.3 deprecation RFC, but only the
> assert_options() function was part of it.
>

Head's up, I'm planning on opening the vote on this on Wednesday the 28th
of June.

Best regards,

George P. Banyard

>

Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

[G. P. B.]

On Wed, 31 May 2023 at 23:20, Claude Pache  wrote:

> Although your RFC says that the `zend.assertions` ini setting has
> superseded `assert.active` for a while, the “official” php.ini file still
> advises to modify the value of `assert.active` rather than the one of
> `zend.assertion` in order to switch behaviour at runtime:
>
>
> https://github.com/php/php-src/blob/91fd5641cde138b8894a48c921929b6e4abd5c97/php.ini-development#L1604
>
> I bet that many people (myself included) follows the advice given in
> `php.ini`.
>

This talks about run-time modification, which is something that I don't
think should be done in practice, nor is often done.


> Another point: Your RFC is missing `assert.quiet_eval`...
>

I have clarified that this was indeed removed in PHP 8.0.
https://wiki.php.net/rfc/assert-string-eval-cleanup

Best regards,

Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

[Claude Pache]

> Le 1 juin 2023 à 00:20, Claude Pache  a écrit :
> 
> Another point: Your RFC is missing `assert.quiet_eval`...
> 

(In fact, it is probable that the `assert.quiet_eval` ini setting and the 
ASSERT_QUIET_EVAL constant have been removed in PHP 8, but that the 
documentation has not been updated.)

—Claude

Re: [PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

[Claude Pache]

> Le 31 mai 2023 à 14:08, G. P. B.  a écrit :
> 
> Hello internals,
> 
> I would like to start the discussion about deprecating various remains from
> the now removed string code evaluated assertions functionality of assert().
> 
> The RFC is located on the wiki at the following address:
> https://wiki.php.net/rfc/assert-string-eval-cleanup
> 
> Initially, this was part of the mass PHP 8.3 deprecation RFC, but only the
> assert_options() function was part of it.
> 
> Best regards,
> 
> George P. Banyard

Hi,

Although your RFC says that the `zend.assertions` ini setting has superseded 
`assert.active` for a while, the “official” php.ini file still advises to 
modify the value of `assert.active` rather than the one of `zend.assertion` in 
order to switch behaviour at runtime:

https://github.com/php/php-src/blob/91fd5641cde138b8894a48c921929b6e4abd5c97/php.ini-development#L1604

I bet that many people (myself included) follows the advice given in `php.ini`.

Another point: Your RFC is missing `assert.quiet_eval`...
  
—Claude

[PHP-DEV] [RFC] Deprecate remains of string evaluated code assertions

[G. P. B.]

Hello internals,

I would like to start the discussion about deprecating various remains from
the now removed string code evaluated assertions functionality of assert().

The RFC is located on the wiki at the following address:
https://wiki.php.net/rfc/assert-string-eval-cleanup

Initially, this was part of the mass PHP 8.3 deprecation RFC, but only the
assert_options() function was part of it.

Best regards,

George P. Banyard