Skip to content

Add missing ZEND constants and descriptions of their PHP aliases #3054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 6, 2024
Merged

Add missing ZEND constants and descriptions of their PHP aliases #3054

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions appendices/reserved.constants.core.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,36 @@
</simpara>
</listitem>
</varlistentry>
<varlistentry xml:id="constant.zend-thread-safe">
<term>
<constant>ZEND_THREAD_SAFE</constant>
(<type>bool</type>)
</term>
<listitem>
<simpara>
Indicates whether the current build of PHP is thread safe.
</simpara>
</listitem>
</varlistentry>
<varlistentry xml:id="constant.zend-debug-build">
<term>
<constant>ZEND_DEBUG_BUILD</constant>
(<type>bool</type>)
</term>
<listitem>
<simpara>
Indicates whether the current build of PHP is a debug build.
</simpara>
</listitem>
</varlistentry>
<varlistentry xml:id="constant.php-zts">
<term>
<constant>PHP_ZTS</constant>
(<type>int</type>)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really an int? And is this is an alias for the ZEND constant? If so maybe saying that it is an alias of &Alias; <constant>NAME</constant> ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the PHP constants are int and represent the same value as the bool ZEND constants (see below for more details). If based on the below you think that I should mark either constant as an alias, just let me know.

The ZEND constants:

Zend/zend_constants_arginfo.h#L26-L27

    REGISTER_BOOL_CONSTANT("ZEND_THREAD_SAFE", ZTS_V, CONST_PERSISTENT);
    REGISTER_BOOL_CONSTANT("ZEND_DEBUG_BUILD", ZEND_DEBUG, CONST_PERSISTENT);

And the PHP constants:

main/main_arginfo.h#L14-L15

	REGISTER_LONG_CONSTANT("PHP_ZTS", PHP_ZTS, CONST_PERSISTENT);
	REGISTER_LONG_CONSTANT("PHP_DEBUG", PHP_DEBUG, CONST_PERSISTENT);

PHP_DEBUG is simply:

main/php.h#L56

#define PHP_DEBUG ZEND_DEBUG

ZEND_DEBUG is defined during PHP compilation.

And ZEND_THREAD_SAFE (ZTS_V) and PHP_ZTS (PHP_ZTS):

main/php_main.h#L61-L69

#ifdef ZTS

#define PHP_ZTS 1
#else
#define PHP_ZTS 0
#endif

Zend/zend_portability.h#L431-L435

#ifdef ZTS
# define ZTS_V 1
#else
# define ZTS_V 0
#endif

ZTS is defined during PHP compilation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, could you mark the ZEND constants as aliases for the PHP ones and mark them all as bool?

I'm wondering if maybe we shouldn't deprecate the Zend ones and only keep the PHP ones, but that is out of scope for this PR (and is not a doc change anyway)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mark the int constants as bool too? I'm not sure how much those are used with strict comparison but if you're sure, I'll mark them as bool.

Of course this doesn't look good either if the docs show all four to be bool: :-)

var_dump(ZEND_DEBUG_BUILD);
var_dump(ZEND_THREAD_SAFE);
var_dump(PHP_DEBUG);
var_dump(PHP_ZTS);
bool(false)
bool(false)
int(0)
int(0)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtf is this... can you file an issue (or PR) on php-src to mention this, as those should be consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll open a PR.

I've searched on GitHub and some popular Composer packages and there doesn't seem to be too many uses of == or === with the PHP_(DEBUG|ZTS) constants.

There is only one of these in php-src:
ext/zip/tests/bug72660.phpt#L7

if(PHP_ZTS == 0) { die('skip ZTS required'); }

It's not an issue but I could change it to the below to be consistent with all the other uses of PHP_ZTS.

if(PHP_ZTS) { die('skip ZTS required'); }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing it to just be if (PHP_ZTS) makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just opened php/php-src#13079 .

Also, I of course meant if (!PHP_ZTS) in my previous message.

</term>
<listitem>
<simpara>

Indicates whether the current build of PHP is thread safe.
</simpara>
</listitem>
</varlistentry>
Expand All @@ -98,7 +120,7 @@
</term>
<listitem>
<simpara>

Indicates whether the current build of PHP is a debug build.
</simpara>
</listitem>
</varlistentry>
Expand Down