JS Codegen issue/improvement

[Christian-W. Budde]

The code below:

type
   JClass = class external
      foo: String;
   end;

function BuildRequest(bar: String): JClass;
begin
   Result := JClass(class
      foo := bar;
   end);
end;

should compile to something like:

function BuildRequest(bar) {
   return {
      "foo" : bar
   };
};

However, it does compile to:

function BuildRequest(bar) {
   var Result = null;
   Result = {
      "foo" : bar
   };
   return Result
};

It’s not that it is incorrect, but too verbose.

Now looking at the DWS code it seems that the optimize block is not really executed:

      // optimize to a straight "return" statement for trivial functions
      if     (not resultIsBoxed) and (proc.Table.Count = 0)
         and ((proc.InitExpr = nil) or (proc.InitExpr.SubExprCount = 0))
         and (proc.Expr is TAssignExpr) then begin

since proc.Table.Count = 1 instead of 0 in most other trivial cases.

Now looking at proc.Table[0] there is an empty symbol (name = ‘’, description = ‘', typ = nil). I’m not sure why it is there in the first place, but as it does nothing it should be removed or at least taken into account like this:

      // optimize to a straight "return" statement for trivial functions
      if     (not resultIsBoxed) and ((proc.Table.Count = 0) or
         ((proc.Table.Count = 1) and (proc.Table[0].Typ = nil)))
         and ((proc.InitExpr = nil) or (proc.InitExpr.SubExprCount = 0))
         and (proc.Expr is TAssignExpr) then begin

With this, the code is again optimized as expected.

[Eric Grange]

The empty symbol is for the anonymous class symbol (the class..end within the cast).

I wonder if rather than looking at the local table symbol count, which holds both local variables and types (and could miss some in case of a sub-scope within a single expression, not possible yet, but could be added to the language later on), it should be possible to check the local stack size instead.

If it is allocated only for the “Result” value (and Result is not “boxed”, ie. not passed as a “var” parameter), I guess it should be safe to allow the “return” optimization even when there are multiple expressions, provided Result is assigned only once, and that assignment is the last expression ?

Edit: an extra condition would be the absence of any form of “exit” in the procedure, and that “Result” is never read-accessed, as in both those case the initialization to a default value would be required.

[Christian-W. Budde]

The suggestion I made (with the additional symbol table check) was not meant to be complete. It was just one possible idea, based on a test. I was not (yet) experienced enough to see possible issues with this approach nor better approaches.

Though, I did think of something like a stack size approach, but I don’t know yet how it is working.

Regarding your thoughts: It should be easy to parse for exit or read access, but we need to be sure that these are the only exceptions then. Again, I’m not experienced enought to foresee other possible issues, but I have a “feeling” that these are not the only two exceptions.

It needs some testing to ensure that it is working sufficiently.

What I can offer at least to test it on my projects. Currently I do have two rather big and stable projects (compiled JS is about 1 MB in size) and several smaller ones. All make use of variants of the above snippet and in case there is something odd, it should probably fail at one end.

[Eric Grange]

Fixed Issue #160, return optimization should work in more cases, and be disabled in some edge cases it should not have been enabled

→ <<cset 50b792759589>>

[Eric Grange]

I went for an intermediate solution: the optimization only still works when there is a single statement, but it now uses stack size and the symbol dictionary, which should be both more flexible and safer (hopefully)