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

[Donald Anderson]

Here's what I get when I run the app.swf directly:

TypeError: Error #1009: Cannot access a property or method of a null  
object reference.
        at LzDelegate/register()
        at $lzc$class_$m207/$m206()
        at Function/http://adobe.com/AS3/2006/builtin::call()
        at LzEvent/sendEvent()
        at LzNode/__LZcallInit()
        at LzCanvas/__LZcallInit()
        at LzCanvas/__LZinstantiationDone()
        at LzInstantiatorService/makeSomeViews()
        at LzInstantiatorService/checkQ()
        at Function/http://adobe.com/AS3/2006/builtin::call()
        at LzEvent/sendEvent()
        at LzIdleService/__idleupdate()
        at LzIdleKernel$/__update()
        at Function/http://adobe.com/AS3/2006/builtin::apply()
        at <anonymous>()
        at SetIntervalTimer/onTimer()
        at flash.utils::Timer/_timerDispatch()
        at flash.utils::Timer/tick()
I guess this gets swallowed when running under the server.  (Is it  
captured to
a log, or would I see that if I ran in debug mode?)

Without this changeset, LzDelegate.register() got translated to:

public function register (eventSender:LzEventable, eventName:String) {
var anEvent = eventSender[eventName];
var m = this.m;
if (m && m.length != 1) {
var a = new Array(m.length);
this.m = function  (ignore:*) {
return m.apply(this, a)
}};
if (!(anEvent is LzEvent)) {
anEvent = new LzEvent(eventSender, eventName, this)
} else {
anEvent.addDelegate(this)
};
this[this.lastevent++] = anEvent
}

With this change, we get this:

public function register ($1:LzEventable, $2:String) {
var $3 = $1[$2];
var m = this.m;
if (m && m.length != 1) {
var a = new Array(m.length);
this.m = function  ($1:*) {
return m.apply(this, a)
}};
if (!($3 is LzEvent)) {
$3 = new LzEvent($1, $2, this)
} else {
$3.addDelegate(this)
};
this[this.lastevent++] = $3
}

I think that's okay.  The function() closure reuses '$1', which actually
isn't an issue in this case since the arg is ignored, but I think it  
points
out a potential shadowing problem.

Also, Tucker mentioned that methods can have withThis turned off.
That might well fix the problem (getting rid of shadowing problems in  
other
places in the code), but I think I want to know for sure that's what  
it is.

Unless you all spot something here, I think I need to get into the  
debugger.
Henry, did you ever put together instructions on how to do that?

Thanks,

- Don

On Jul 17, 2008, at 10:16 AM, Henry Minsky wrote:

I can help debug swf9 tests, if you need.  Just ping me if you would
like me to look at something that's failing.




On Thu, Jul 17, 2008 at 9:39 AM, Donald Anderson  
<[EMAIL PROTECTED]> wrote:

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 [EMAIL PROTECTED] 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: [EMAIL PROTECTED]

www: http://www.ddanderson.com






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

voice: 617-547-7881
email: [EMAIL PROTECTED]
www: http://www.ddanderson.com







--
Henry Minsky
Software Architect
[EMAIL PROTECTED]


--

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

voice: 617-547-7881
email: [EMAIL PROTECTED]
www: http://www.ddanderson.com