Oh, regarding the optimizations, if any composite expression has a non-literal in it, then it is not a literal. So the most important thing you can do is to merge the static parts together as much as possible unless they have a dynamic bit between them.
EDIT: It is also likely better if you emit: {:__block__, [], [..., ..., ..., {:safe, [static, dynamic, ...]}]}. At runtime it should be the same but it may make compilation slightly faster.
I’m already merging the static parts together.
I was talking about having a specific engine which doesn’t even reference the static parts to save memory (and maybe some CPU cycles).
At runtime, evaluating the dynamic parts directly seems more efficient than creating a list with both static and dynamic parts and then filtering out the static parts, especially in terms of memory consumption.
Ok, I can compare both possibilities. This sounds like a great use case for my schism library 
(I’ve specifically written it to compare two implementations with just a small change deep into the calling hierarchy, which is what we have here).
But you do understand what the problem is with expanding a macro outside of its quoted expression, right? Suppose you have something like this:
<% a = 1 %>
<%= bad_macro(a) %>
Where bad_macro/1 uses something like Code.eval_quoted(a), which needs a to be available. Macros like this are not very common, of course. If you expand the expression in isolation, you will get an error, which I think is inconsistent with normal EEx semantics.
EDIT: in “experiment land” I can expand the macros like you suggest, of course.
I agree the separation should be done at compile time. What I am saying though is that the separation could be done outside of the engine. Basically, the engine focuses on emitting the AST in a given format and a next step can go ahead and separate the bits, without having to introduce a whole new engine.
So you’re suggesting something like:
defmodule PhoenixUndeadView.TemplateCompiler do
def compile_full(text, env) do
EEx.compile_string(text, engine: UndeadEngineFull, opts: [env: env])
end
def compile_static(text, env) do
text
|> EEx.compile_string(engine: UndeadEngineFull, opts: [env: env])
|> extract_static()
end
def compile_dynamic(text, env) do
text
|> EEx.compile_string(engine: UndeadEngineFull, opts: [env: env])
|> extract_dynamic()
end
end
Or something even more optimized such as:
defmodule PhoenixUndeadView.OptimizedTemplateCompiler do
def compile(text, env) do
full = EEx.compile_string(text, engine: UndeadEngineFull, opts: [env: env])
static = extract_static(full)
dynamic = extract_dynamic(full)
%{full: full, static: static, dynamic: dynamic}
end
end
In that case I agree
The UndeadEngine is now an implementation detail inaccessible to the user. The user should never have to deal with the engine directly.
The main entry point is the UndeadEEx module, which exports the UndeadEEx.compile_string/2 function. This function compiles the text into a PhoenixUndeadView.%UndeadTemplate{}, which contains quoted expressions for the full template, the static parts and the dynamic parts.
iex> alias PhoenixUndeadView.UndeadEEx
PhoenixUndeadView.UndeadEEx
iex> UndeadEEx.compile_string(text, env: __ENV__)
%PhoenixUndeadView.UndeadTemplate{
# Quoted expression for the full template
full: ...,
# Quoted expression for the static parts
static: ...,
# Quoted expression for the dynamic parts
dynamic: ...
}
The implementation is what you suggest. I generate the full template and then remove the static or dynamic parts accordingly. To increase the efficiency, the literal binaries now appear in the final list. The template now returns this:
{:safe, ["", dynamic__1, "\nBlah blah blah\n", dynamic__2, "\nBlah blah\n", dynamic__3, ""]}
instead of this:
{:safe, [static__1, dynamic__1, static__2, dynamic__2, static__3, dynamic__3, static__4]}
Besides being cleaner, it makes the return values easier to analyze statically. It’s now obvious what is static and what is dynamic: everything that’s a literal string is static; everything else is dynamic.
I still haven’timplemented macro expansion… I’ll do so when I have a bigger stretch of free time.
EDIT: you can look at the implementation in the repo I’ve linked.
Just a note regarding some easy optimizations:
The following return value:
{:safe, ["", dynamic__1, "\nBlah blah blah\n", dynamic__2, "\nBlah blah\n", dynamic__3, ""]}
Could be optimized further into:
{:safe, [dynamic__1, "\nBlah blah blah\n", dynamic__2, "\nBlah blah\n" | dynamic__3]}
by removing the empty binaries and turning it into something that’s no longer a proper list (iolists don’t need to be proper lists). I’m not doing this because it’s easier to work on a list if the list is proper starts and ends with a binary (for other reasons I don’t want to explain right now). After all other optimizations have been performed, we can optimize it even further by performing the transformation above.
Thankfully the BEAM doesn’t copy them into the list, it only references them from the global storage.
Cool. But I think it’s still worth it to filter them out at compile time inestead of doing it at runtime.