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

Broken enforced output append optimization #3

Open
segabor opened this issue Oct 29, 2018 · 0 comments
Open

Broken enforced output append optimization #3

segabor opened this issue Oct 29, 2018 · 0 comments
Assignees
Labels

Comments

@segabor
Copy link
Owner

segabor commented Oct 29, 2018

The following template code

  {for $name in $names}
    {call .helloName}
      {param name: $name /}
    {/call}
    {if not isLast($name)}
      <br>  // break after every line except the last
    {/if}
  {/for}

compiles to

output.extend(helloName(["name": nameData30], ijData).description+(! (nameIndex30 == nameList30.endIndex-1)) ? "<br>")

Swift code generation was derived from the Python compiler that enforces inlining output appends. First this looks pretty awful, second it's probably inefficient.
Let's rewrite the compiler to generate a more readable code like

output.append(helloName(["name": nameData30], ijData).description)
if nameIndex30 != nameList30.endIndex {
  output.append("<br>")
}
@segabor segabor added the bug label Oct 29, 2018
@segabor segabor self-assigned this Oct 29, 2018
segabor pushed a commit that referenced this issue Jun 18, 2019
The addition of @State and default parameters has stretched TemplateVariableManager past its breaking point.  It currently has 3 responsibilities

1. creating field definitions
2. managing local variable lifetimes
3. generating save/restore logic

The issue is that #1 is for the whole class, #2 is for a single method and #3 is for the render() method.

This conflation was fine when the render() method was the only method with any non-trivial logic, but now that we have default parameters and @State initializers we have a desire to generate code for arbitrary soy expressions in the constructor or static initializers.  Right now this is just corrupting some debugging metadata, but future changes will cause more issues.

This requires us to break up these responsibilities.  This is step 1.  next will be to extract a mechanism to create temporary variables that is detatched from save/restore state solution.

GITHUB_BREAKING_CHANGES=none

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=249533688
segabor pushed a commit that referenced this issue Apr 7, 2020
…is involves

1. making more comments LineCommentLeaf nodes
2. removing inlining of attribute values if the value contains comments (or is force)
3. creating leaves instead of finding the nearest appending point if the first or last element is a LineCommentLeaf.

The optimization in #3 was nice, but it made the formatting idempotent since it changes the AST in a way that it would format differently a second time around.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=301222043
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant