Skip to content

Prefix all sprintf() calls #57461

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
Jun 21, 2024
Merged

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Jun 20, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? no
Deprecations? no
Issues N/A
License MIT

see https://tideways.com/profiler/blog/new-in-php-8-4-engine-optimization-of-sprintf-to-string-interpolation

In PHP 8.4, some calls to sprintf() will be optimized by the compiler. I therefore propose to change our code style and force calls to that function to be prefixed with a \. This will tell the compiler that this function must not be looked up in the current namespace first and allow the compiler to optimize the call.

The changes have been automated with PHP CS Fixer.

@derrabus
Copy link
Member Author

cc @beberlei

@derrabus derrabus force-pushed the improvement/prefixed-sprintf branch 2 times, most recently from f4f2588 to f94ea7f Compare June 20, 2024 08:34
@derrabus derrabus force-pushed the improvement/prefixed-sprintf branch from f94ea7f to b6661b7 Compare June 20, 2024 08:40
@MatTheCat
Copy link
Contributor

Should .git-blame-ignore-revs be updated?

@derrabus derrabus force-pushed the improvement/prefixed-sprintf branch from b6661b7 to 6ce530c Compare June 20, 2024 15:52
@derrabus
Copy link
Member Author

Should .git-blame-ignore-revs be updated?

Will do, as soon as this PR is merged and the commit hash is final.

@fabpot
Copy link
Member

fabpot commented Jun 21, 2024

Thank you @derrabus.

@fabpot fabpot merged commit dcd2219 into symfony:7.2 Jun 21, 2024
4 of 11 checks passed
@derrabus derrabus deleted the improvement/prefixed-sprintf branch June 21, 2024 07:21
@derrabus
Copy link
Member Author

Should .git-blame-ignore-revs be updated?

Will do, as soon as this PR is merged and the commit hash is final.

#57472

derrabus added a commit that referenced this pull request Jun 21, 2024
This PR was merged into the 7.2 branch.

Discussion
----------

Add CS commit to .git-blame-ignore-revs

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | Follows #57461
| License       | MIT

Let's ignore 6ce530c from #57461 when blaming changes.

Commits
-------

2686d9d Add CS commit to .git-blame-ignore-revs
smnandre added a commit to smnandre/ux that referenced this pull request Jun 22, 2024
smnandre added a commit to smnandre/ux that referenced this pull request Jun 22, 2024
kbond added a commit to symfony/ux that referenced this pull request Jun 26, 2024
This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[CS] Optimize `sprintf` calls for PHP 8.4

Follows
* symfony/symfony#57461
* twigphp/Twig#4108

Update php-cs-fixer config + apply rule on all the repository code

Commits
-------

eab7db5 [CS] Optimize `sprintf` calls for PHP 8.4
Sign up for free to 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