Skip to content
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.

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

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

Merged
merged 1 commit into from
Jan 6, 2024

Conversation

haszi
Copy link
Contributor

@haszi haszi commented Jan 1, 2024

Added the missing constants (#2747) ZEND_THREAD_SAFE and ZEND_DEBUG_BUILD, and descriptions of their PHP integer aliases (PHP_ZTS and PHP_DEBUG).

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.

@Girgias Girgias merged commit 140689d into php:master Jan 6, 2024
2 checks passed
@haszi haszi deleted the Add-missing-ZEND-constants branch January 6, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants