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

Rely on reflection to access null properties #4425

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

nicolas-grekas
Copy link
Contributor

@nicolas-grekas nicolas-grekas commented Oct 29, 2024

Benchmark like https://gist.github.com/arnaud-lb/c4235164fb41e11672aa670da8fa4fc7 (which were done for another use case but is still relevant here) prove that using reflection instead of array-casts is significantly faster for accessing properties.

Checking for dynamic properties is done with property_exists.

@nicolas-grekas nicolas-grekas changed the title Rely on reflection to access properties Rely on reflection to access null properties Oct 29, 2024
src/Extension/CoreExtension.php Outdated Show resolved Hide resolved
src/Extension/CoreExtension.php Outdated Show resolved Hide resolved
@nicolas-grekas nicolas-grekas force-pushed the array-cast-less branch 2 times, most recently from c186115 to d8f39e6 Compare October 31, 2024 12:02
@nicolas-grekas
Copy link
Contributor Author

PR updated with a simpler and more performant approach, that makes DateTimeInterface a special case (because it is).

@nicolas-grekas nicolas-grekas force-pushed the array-cast-less branch 2 times, most recently from 5e1b6fe to ca54737 Compare October 31, 2024 13:52
@fabpot
Copy link
Contributor

fabpot commented Nov 7, 2024

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 0232d38 into twigphp:3.x Nov 7, 2024
46 of 49 checks passed
@@ -1642,7 +1642,7 @@ public static function getAttribute(Environment $env, Source $source, $object, $

// array
if (Template::METHOD_CALL !== $type) {
$arrayItem = \is_bool($item) || \is_float($item) ? (int) $item : $item;
$arrayItem = \is_bool($item) || \is_float($item) ? (int) $item : $item = (string) $item;
Copy link
Contributor

Choose a reason for hiding this comment

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

The string casting breaks my ArrayAccess object using objects as keys, when using myCollection[object] in twig.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arf. Can you please submit a fix as a PR together with a test case?

@nicolas-grekas nicolas-grekas deleted the array-cast-less branch December 4, 2024 10:08
fabpot added a commit that referenced this pull request Dec 10, 2024
This PR was merged into the 3.x branch.

Discussion
----------

Fix ArrayAccess with objects as keys

fixes #4476
The bug was introduced in 3.15 via #4425 (comment)

Commits
-------

32a75b7 Fix ArrayAccess with objects as keys
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.

5 participants