Skip to content

Replace @deprecated with #[\Deprecated] for internal constants - #18780

Merged
DanielEScherzer merged 3 commits into
php:masterfrom
DanielEScherzer:constants-stubs-use-deprecated
Jun 26, 2025
Merged

Replace @deprecated with #[\Deprecated] for internal constants#18780
DanielEScherzer merged 3 commits into
php:masterfrom
DanielEScherzer:constants-stubs-use-deprecated

Conversation

@DanielEScherzer

@DanielEScherzerDanielEScherzer commented Jun 5, 2025

Copy link
Copy Markdown
Member

Only covers constants declared via stub files, others will be handled separately in a later commit.

Does not include the intl extension, since that had some errors relating to the cpp code; that extension will be updated separately.

Also fix an undefined variable warning in gen_stub.php.

@ndossche

Copy link
Copy Markdown
Member

If you look at the filter arginfo you can see that the same strings are allocated twice. Do you think you can avoid this at least within the same arginfo file?

Comment threadext/random/random.stub.php Outdated
@DanielEScherzer

Copy link
Copy Markdown
MemberAuthor

If you look at the filter arginfo you can see that the same strings are allocated twice. Do you think you can avoid this at least within the same arginfo file?

Not easily - I'll add it to my list of improvements for gen_stub.php, but this isn't anything new

@TimWollaTimWolla left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok for ext/random

@TimWolla

Copy link
Copy Markdown
Member

the same strings are allocated twice

It should probably be safe to just intern them all. They are always allocated on start-up, so the memory will be used either way. Interning should just reduce it, no?

@DanielEScherzer

Copy link
Copy Markdown
MemberAuthor

the same strings are allocated twice

It should probably be safe to just intern them all. They are always allocated on start-up, so the memory will be used either way. Interning should just reduce it, no?

Probably, and I will definitely look into this, but I'd prefer not to block the rest of the patch

@ndossche

Copy link
Copy Markdown
Member

the same strings are allocated twice

It should probably be safe to just intern them all. They are always allocated on start-up, so the memory will be used either way. Interning should just reduce it, no?

Yes I agree.
If you pinky promise that you're gonna look at it before 8.5's release I'll allow it.

@ndosschendossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK for DOM, Enchant, SOAP

@DanielEScherzer

Copy link
Copy Markdown
MemberAuthor

the same strings are allocated twice

It should probably be safe to just intern them all. They are always allocated on start-up, so the memory will be used either way. Interning should just reduce it, no?

Yes I agree. If you pinky promise that you're gonna look at it before 8.5's release I'll allow it.

Pinky promise - I have #18665 and a few other bits of cleanup in mind that shouldn't change any of the generated arginfo files, and then I'll address the improvements that will affect arginfo (like adding interned strings)

@DanielEScherzer

DanielEScherzer commented Jun 9, 2025

Copy link
Copy Markdown
MemberAuthor

Code owner approvals:

  • curl - adoy
  • date - derickr
  • dom - nielsdos
  • enchant - No owner or current maintainer (approved by neilsdos)
  • filter - No owner or current maintainer
  • mysqli - bukka / kamil-tekiela (approved by bukka)
  • pgsql - devnexen
  • random - timwolla / zeriyoshi (approved by tim wolla)
  • reflection - me
  • soap - neilsdos
  • standard - bukka

Comment threadext/soap/soap.stub.php Outdated
Comment threadbuild/gen_stub.php Outdated
Comment threadext/mysqli/mysqli.stub.php Outdated
@DanielEScherzer

Copy link
Copy Markdown
MemberAuthor

Barring any further review I plan to merge this in the next few days

DanielEScherzerand others added 3 commits June 26, 2025 10:46
Only covers constants declared via stub files, others will be handled
separately in a later commit.
Does not include the intl extension, since that had some errors relating to the
cpp code; that extension will be updated separately.
Also fix an undefined variable warning in gen_stub.php.
This is already fixed differently in master.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@DanielEScherzer@ndossche@TimWolla@bukka@kocsismate@Girgias