Fixed optimization of filters and misoptimizations with changed eval context#476
Fixed optimization of filters and misoptimizations with changed eval context#476snoack wants to merge 3 commits intopallets:masterfrom snoack:fix-optimizer
Conversation
|
I suspect you haven't gotten much feedback simply because there's a lot going on here. I don't have the time to review it in depth either right now. Since none of these change external APIs (other than fixing bugs), then if you are confident that these are still good, I'd say go ahead and merge it, along with #478 and #479 since they all seem to be related. |
|
I'm having trouble merging and evaluating this. The first commit is already applied in master, and no tests fail, or appear to have been changed along with that commit. While keeping the context during optimization seems correct, I don't know the original intention for not doing this. I cannot get @snoack if you can submit a new PR that merges and has tests, that would be appreciated. |
I realized that some filters aren't inlined during optimizations. It turned out that this effects all environment and eval context filters. That is because the environment or eval context isn't passed as first but second argument. Therefore the filter function raises an exception and the optimizer skips the node. This one was easy to fix, see the first commit in this PR.
However, now things get funny. With that fix, the tests for the
{% autoescape %}tag suddenly start to fail. As it turned out the optimizer were agnostic to the eval context, and theas_const()method created implicitly a new eval context for every node, ignoring{% autoescape %}tags up the tree. So I fixed this as well, see the second commit in this PR. Therefore optimizer now keeps track of the eval context, and updates it when visitingScopedEvalContextModifiernodes. To avoid future issues like that and to get rid of dead code, I also made the eval context passed toas_const()mandatory.