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

Add VariableOptimizerNodeVisitor #4527

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

fabpot
Copy link
Contributor

@fabpot fabpot commented Jan 2, 2025

This is the simplest useful node visitor that use the new types information to optimize the compiled code.

@fabpot
Copy link
Contributor Author

fabpot commented Jan 3, 2025

I think that this node visitor must also enforce the availability of the variable at runtime to make this works correctly.
Like converting the types tag to checks that would throw exceptions based on the existence of the variables and maybe checking the types as well. The more I think about it, the more I think that this would be like a "strict" mode that would enable these performance optimizations.

WDYT /cc @jdreesen @ruudk?

@jdreesen
Copy link
Contributor

jdreesen commented Jan 3, 2025

Did you mean @drjayvee instead?

@fabpot
Copy link
Contributor Author

fabpot commented Jan 3, 2025

Did you mean @drjayvee instead?

Oops, sorry for the wrong ping.

@drjayvee
Copy link
Contributor

drjayvee commented Jan 6, 2025

This is the simplest useful node visitor that use the new types information to optimize the compiled code.

Sure, but it does add an exception for strict_variables, namely that typed variables never default to null even if strict_variables is disabled.

I think that this node visitor must also enforce the availability of the variable at runtime to make this works correctly
Like converting the types tag to checks that would throw exceptions based on the existence of the variables and maybe checking the types as well. The more I think about it, the more I think that this would be like a "strict" mode that would enable these performance optimizations.

When you say "checking the types", I'm assuming you mean a run-time check? The other meaning is compile-time validation of the type to see whether it is valid, and whether a class (or interface, trait) exists.

Sorry, I didn't read your comment properly.

Aside from the run-time checks, there's also the (possible) compile-time validation of the type to check whether it is valid, and whether a given class (or interface, trait) exists.

TwigQI does all of those already. I wouldn't mind bringing some of its functionality into Twig core.

However, all of these require that we first standardize the typing system and syntax. That discussion in #4532 proved to be controversial. I wouldn't mind having another go at it, however.

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