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

Replace strtolower() with strtr() when dealing with method names #3237

Merged
merged 1 commit into from
Jan 8, 2020

Conversation

mpdude
Copy link
Contributor

@mpdude mpdude commented Jan 8, 2020

This is to improve consistency with the implementation of case-insensitivity for method names etc. in the Zend Engine: In fact, the ZE does not apply the internal strtolower implementation, but just maps ASCII A-Z to a-z.

This was done in php/php-src@582514d to fix issues like https://bugs.php.net/bug.php?id=18556.

Additionally, strtok might give a tiny (measurable?) performance gain over strtolower, plus it is not locale-dependant.

@mpdude
Copy link
Contributor Author

mpdude commented Jan 8, 2020

⚠️ Tests are green, but in fact some of the jobs did not really execute. See symfony/symfony#35254.

@fabpot fabpot force-pushed the consistent-lowercase branch from 40bb530 to 30fedde Compare January 8, 2020 08:27
@fabpot
Copy link
Contributor

fabpot commented Jan 8, 2020

Thank you @mpdude.

fabpot added a commit that referenced this pull request Jan 8, 2020
…names (mpdude)

This PR was squashed before being merged into the 1.x branch (closes #3237).

Discussion
----------

Replace strtolower() with strtr() when dealing with method names

This is to improve consistency with the implementation of case-insensitivity for method names etc. in the Zend Engine: In fact, the ZE does not apply the internal `strtolower` implementation, but just maps ASCII `A-Z` to `a-z`.

This was done in php/php-src@582514d to fix issues like https://bugs.php.net/bug.php?id=18556.

Additionally, `strtok` might give a tiny (measurable?) performance gain over `strtolower`, plus it is not locale-dependant.

Commits
-------

30fedde Replace strtolower() with strtr() when dealing with method names
@fabpot fabpot merged commit 30fedde into twigphp:1.x Jan 8, 2020
@mpdude mpdude deleted the consistent-lowercase branch January 8, 2020 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants