[Laszlo-dev] For Review: Change 20080716-dda-V Summary: Fix remapLocals and withThis in SWF9

[Donald Anderson]

On Jul 17, 2008, at 9:10 AM, P T Withington wrote:

> Questions:
>
> 1) Do you need to copy default values for parameters, or is that not  
> stored on the parameter the way type and ellipsis is?

I think it already happens.   Parameter names are ASTIdentifiers and  
initializers are anything else (maybe ASTLiteral,
or an expression tree).  This loop changes the ASTIdentifiers and  
preserves the rest:

     // Replace params
     for (int i = 0, len = paramIds.length; i < len; i++) {
       if (paramIds[i] instanceof ASTIdentifier) {
         ASTIdentifier oldParam = (ASTIdentifier)paramIds[i];
         SimpleNode newParam = translateReference(oldParam).declare();
         params.set(i, newParam);
       }
     }

But I'll find a way to test this.


> 2) The tag compiler will emit #pragma withThis in both functions and  
> methods.  The either the swf9 back-end should turn that into a no-op  
> in a method (as it is redundant), or we can change the tag compiler  
> to _not_ emit that in methods, but then the swf8 and dhtml back-ends  
> will have to automatically insert it for methods.  I believe this  
> will simply be an efficiency not a correctness issue.  If you want  
> to defer, please file a separate bug.  The only way I know to test  
> this would be to inspect the generated code.

Should be easy enough.  We are already tracking when we are in a  
declared class and also when we are in a declared method.
Is that enough?  Do any closures (which might have #pragma withThis)  
occur within a Class.Method body?

> 3) I'd like to make sure at least _one_ of the known working swf9  
> tests works after this change.  If hello is the test, we need to get  
> that working again first.

Agreed.  Max got lzpix working, and it does not yet work with this  
changeset, so I have some work to do to debug that.

>
>
> Otherwise, approved.

Thanks, I'll incorporate these and if the changes after debugging are  
non-trivial, I will have you review again.

>
>
> On 2008-07-16, at 23:10EDT, Donald Anderson wrote:
>
>> Change 20080716-dda-V by dda at 97.sub-75-192-214.myvzw.com on  
>> 2008-07-16 21:26:41 EDT
>>   in /Users/dda/laszlo/src/svn/openlaszlo/trunk-a
>>   for http://svn.openlaszlo.org/openlaszlo/trunk
>>
>> Summary: Fix remapLocals and withThis in SWF9
>>
>> New Features:
>>
>> Bugs Fixed: LPP-6637
>>
>> Technical Reviewer: ptw (pending)
>> QA Reviewer: (pending)
>> Doc Reviewer: (pending)
>>
>> Documentation:
>>
>> Release Notes:
>>
>> Details:
>>   The SWF9 code generator has had a long standing bug that prevented
>>   the withThis pragma from working.  This change set fixes the  
>> fundamental bug and
>>   other smaller problems in order to allow withThis to function  
>> properly.
>>   withThis is necessary for the new approach to handling states.
>>
>>   The code that disables the withThis from being operation has been  
>> removed.
>>
>>   The bug was an extra visit of the function - the second visit  
>> sees some
>>   artifacts left by the first visit, and gets confused.  This has  
>> been fixed.
>>
>>   Mixins in SWF9 are handled by keeping the AST for the mixin and  
>> visiting
>>   it whenever it is mixed in.  The visited AST was not a complete  
>> copy,
>>   only a shallow copy, and this exposed the same sort of extra  
>> visit bug.
>>   A deep copy function for SimpleNode (the basis for AST) was added.
>>   This required minor changes to all existing handcoded AST classes  
>> to
>>   support deep copy.  Fortunately there are not many of these  
>> classes,
>>   but if we add any, we'll need to implement the deepCopy() function.
>>
>>   Remapped formal arguments, (e.g. function foo(a, b) => function  
>> foo($1, $2)),
>>   must retain any ellipsis from the original declaration (e.g.  
>> function foo(a, ...b) => function foo($1, ...$2))
>>   I am also retaining type info, (e.g. function foo(a:*, b) =>  
>> function foo($1:*, $2))
>>
>> Tests:
>>  Passes test case given by ptw in LPP-6637.
>>  Builds LFC without errors with withThis code enabled.
>>  Regression tests: smokecheck for swf8/dhtml.
>>  Unfortunately SWF9 hello seems broken (not due to this change),
>>  so unable to test that.
>>
>> Files:
>> M      WEB-INF/lps/server/src/org/openlaszlo/sc/ 
>> JavascriptGenerator.java
>> M      WEB-INF/lps/server/src/org/openlaszlo/sc/SWF9Generator.java
>> M      WEB-INF/lps/server/src/org/openlaszlo/sc/Compiler.java
>> M      WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/ 
>> SimpleNode.java
>> M      WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/ 
>> ASTModifiedDefinition.java
>> M      WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/ 
>> ASTOperator.java
>> M      WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/ 
>> ASTLiteral.java
>> M      WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/ 
>> ASTFormalParameterList.java
>> M      WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/ 
>> ASTIdentifier.java
>> M      WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/ 
>> ASTPassthroughDirective.java
>>
>> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20080716-dda-V.tar
>>
>>
>>
>> --
>>
>> Don Anderson
>> Java/C/C++, Berkeley DB, systems consultant
>>
>> voice: 617-547-7881
>> email: dda at ddanderson.com
>> www: http://www.ddanderson.com
>>
>>
>>
>


--

Don Anderson
Java/C/C++, Berkeley DB, systems consultant

voice: 617-547-7881
email: dda at ddanderson.com
www: http://www.ddanderson.com



-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.openlaszlo.org/pipermail/laszlo-dev/attachments/20080717/b126e45a/attachment-0001.html