Skip to content

Do not generate CONST_CS when registering constants #9439

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

Merged
merged 1 commit into from
Aug 28, 2022

Conversation

kocsismate
Copy link
Member

@kocsismate kocsismate commented Aug 27, 2022

CONST_CS hasn't been in use for quite a while.

Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

LGTM as per #9135 (review) 😃

@kocsismate kocsismate merged commit b4ec3e9 into php:master Aug 28, 2022
@kocsismate kocsismate deleted the remove-cost-cs branch August 28, 2022 06:27
@TysonAndre
Copy link
Contributor

TysonAndre commented Oct 7, 2022

build/gen_stubs.php is also used by some PECL extensions or being encouraged to be used. Extension authors using gen_stub.php (or adopting it for the first time) might miss the effect this has when their extensions are built for php 7.4 and below - existing tests would pass, and end users would only notice if they attempted to use the wrong case for the constant.

There's also no way to force build/gen_stubs.php to emit CONST_CS (e.g. even for the legacy arginfo flags)

» php -a
Interactive shell

php > var_export(SOME_CONSTANT_FROM_STUBS);
283
php > var_export(some_constant_from_stubs);
Deprecated: Case-insensitive constants are deprecated. The correct casing for this constant is "SOME_CONSTANT_FROM_STUBS" in php shell code on line 1
283

Maybe there could be a .php_stubs_config.json file (or properties/env style file) and this could check all parent directories, if we want to apply certain settings that only work in php 8.3+ to all folders in php-src. Or just the existence of a .php_stubs_use_latest file, though a full config file would be better for customizations if we wanted them in the future

@kocsismate
Copy link
Member Author

Thank you for bringing the issue to my attention! I retained the original behavior for PHP 7.x in 69ef324.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment