JS Codegen: Double initialization of variables

[Christian-W. Budde]

Compiling and JS code gen'ing a simple script like:

var Foo := False;
var Bar := '';

will result in a JS like this:

var Foo=false,Bar="";Foo=false;Bar="";

While it makes sense why this happens (first the varialbes are initialized with their default value and then the given value is assigned), it is totally useless in real life as it doesn't make the code faster or better.

The solution to write Pascal code such as:

var Foo: Boolean;
var Bar: String;

works, but makes it less readable as the reader must be aware that the default value for a boolean is false and for a string is an empty string.

A solution for the problem would be to check if the assigned value is already the default value. In that case the additional assign can be omitted.

It's just a small thing, but so far it will generate warnings, when the generated JS code is recompiled with Google's closure compiler.

[Christian-W. Budde]

How about adding something like this:

// CreateNamedVarDeclExpr
//
function TdwsCompiler.CreateNamedVarDeclExpr(...);
...
begin
...
      Result:=CreateAssign(scriptPos, ttASSIGN, varExpr, initExpr);
      if Optimize and Assigned(Result) then
      begin
        if (TAssignExpr(Result).Right is TConstBooleanExpr) and TConstBooleanExpr(TAssignExpr(Result).Right).Value = False then
        begin
          Result.Free;
          Result := nil;
        end;
      end;

      initExpr:=nil;
...
end;

Probably not the best solution, but rather an idea...

[Christian-W. Budde]

It can get even worse. If the code is in a different unit like this:

unit UnitFooBar;

interface

implementation

var Foo := False;
var Bar := '';

end.

The variables are declared twice:

var Foo=false,Bar="";var Foo=false;var Bar="";

These double declarations will not only raise warnings, but errors in the closure compiler (if not deactivated as in the online version).

It's not critical, but again unnecessary.

[Christian-W. Budde]

A reason for the latter issue is due to the fact that the "right" side of the TAssignExpr has been transformed to a TAssignConstTo*VarExpr, which needs to get handled separately.

Similar to the above solution something like:

      ...
      Result:=CreateAssign(scriptPos, ttASSIGN, varExpr, initExpr);
      if Optimize and Assigned(Result) then
      begin
        if ((Result is TAssignConstToBoolVarExpr) and (TAssignConstToBoolVarExpr(Result).Right = False)) or
           ((Result is TAssignConstToStringVarExpr) and (TAssignConstToStringVarExpr(Result).Right = '')) or
           ((Result is TAssignConstToIntegerVarExpr) and (TAssignConstToIntegerVarExpr(Result).Right = 0))or
           ((Result is TAssignConstToFloatVarExpr) and (TAssignConstToFloatVarExpr(Result).Right = 0)) then
        begin
          Result.Free;
          Result := nil;
        end
        else
        if ((TAssignExpr(Result).Right is TConstBooleanExpr) and (TConstBooleanExpr(TAssignExpr(Result).Right).Value = False)) or
           ((TAssignExpr(Result).Right is TConstStringExpr) and (TConstStringExpr(TAssignExpr(Result).Right).Value = '')) or
           ((TAssignExpr(Result).Right is TConstIntExpr) and (TConstIntExpr(TAssignExpr(Result).Right).Value = 0)) or
           ((TAssignExpr(Result).Right is TConstFloatExpr) and (TConstFloatExpr(TAssignExpr(Result).Right).Value = 0)) then
        begin
          Result.Free;
          Result := nil;
        end;
      end;
      ...

can do the trick.

I haven't done a sufficient amount of tests, but the above code seems to work around the issues.

Still, I think the double 'var ' for non-default cases doesn't make sense and it still remains unclear why the assign expressions are sometimes raw and sometimes somewhat optimized (I'm pretty sure that I have used the very same compiler switches both times).

[Eric Grange]

Hmmm, with your first snippet I get the following output:

var Foo = false,
   Bar = "";
var $dws = function() {
   Foo = false;
   Bar = "";
}
$dws();

Which codegen options are you using?

The first is for static variable & type "declaration", the second is for re-initializing state upon execution.

[Christian-W. Budde]

In the project where I noticed this behaviour I'm using it with an empty MainBodyName. This is because the code is rather a library type which doesn't really need a dedicated namespace as most (if not all) functions are "exported" anyway.

[Eric Grange]

Committed some changes, including some merging of "var" declarations, though there can still be duplicate assignments as there is not attempt to analyze possible initialization dependencies (or lack of). Next step would be to merge the $temp vars (used for loop boundaries), and merge/reordre constant assignments, so that a

var a = 0, b = "", c = 0, b = "hello";

can be codegen'ed as

var a = c = 0, b = "hello";

It does not address your case specifically (with an empty MainBodyName) however.