[Laszlo-dev] For Review: Change 20090111-dda-y Summary: Remove references to 'this' in static functions (revised 2)

Sun Jan 11 21:36:43 PST 2009

On Jan 11, 2009, at 1:52 PM, P T Withington wrote:

> I think here:
>
>>    boolean withThis = options.getBoolean(Compiler.WITH_THIS) && ! 
>> isStatic;
>
> You should be saying:
>
>    // If the locals are not remapped, we assume we are in a runtime
>    // that already does implicit this in methods...
>    boolean withThis = options.getBoolean(Compiler.WITH_THIS) && ! 
> isStatic && remaplocals();
>
> (removing the remaplocals() test from here:
>
>>    if ((! free.isEmpty()) && withThis && remapLocals()) {
>
> where it is now redundant.  Basically, you turn off withThis if you  
> already have implicit this, right?
>
> And here:
>
>>        if (isStatic) {
>>          frag += " { _1 }}).call(null);";
>>        }
>>        else {
>>          frag += " with (this) { _1 }}).call(this);";
>>        }
>
> you should be using `withThis` not `isStatic`, otherwise you are  
> unnecessarily inserting a `with (this)` in the try closure.

Although these seem good to me, if I make these changes, all the SWF9  
applications I create come up blank.
I can investigate tomorrow.

>
>
> And, if you don't need to wrap a `with (this)` around the body,  
> remind me again why the body has to be put in a closure at all?

It doesn't strictly have to be a closure, but this seemed the cleanest  
way to preserve the behavior of return statements.
And if there is no 'return' statement and one is required, we want to  
issue an error just like SWF9 does.
I have annotated LPP-7514 with the chain of thought and test cases  
that led to the use of closures.

- Don

> On 2009-01-11, at 11:14EST, Donald Anderson wrote:
>
>> Change 20090111-dda-y by dda at lester.local on 2009-01-11 10:54:15 EST
>>   in /Users/dda/laszlo/src/svn/openlaszlo/trunk-c
>>   for http://svn.openlaszlo.org/openlaszlo/trunk
>>
>> Summary: Remove references to 'this' in static functions (revised 2)
>>
>> New Features:
>>
>> Bugs Fixed: LPP-7514 (Compilation error when setting  
>> compiler.swf9.catcherrors=true in lps.properties)
>>
>> Technical Reviewer: ptw (pending)
>> QA Reviewer: (pending)
>> Doc Reviewer: (pending)
>>
>> Documentation:
>>
>> Release Notes:
>>
>> Details:
>>   ** This changeset under consideration for 4.2.0.1
>>
>>   [This changeset is a second revision of an earlier changeset, to  
>> ensure the new test case
>>    listed in the JIRA runs correctly.  Everything indicated under  
>> Tests has been retested with this revision.]
>>
>>   The implementation of catchFunctionExceptions (catcherrors) was  
>> blind to static functions,
>>   'this' cannot be used in static functions.
>>
>>   To tell that a function is static, this changeset peeks up the  
>> tree to the parent
>>   of the function node, which should be a ASTModifiedDefinition  
>> ('moddef'), and checks the static
>>   flag there.  A shortcut in copying part of the tree without the  
>> moddef
>>   needed to be fixed, to do that, a 'shallow copy' function was  
>> added to the moddef class.
>>
>>   An alternate way of detecting static-ness is to set a 'static'  
>> property in the options.
>>   I had tried in addressing LPP-5813 (not yet completed), and had  
>> some trouble getting
>>   it to work.  It's not clear which one is cleaner, but the one  
>> that works wins for me.
>>
>>   This fix is slightly more encompassing than it needs to be, it  
>> also disables any possibility
>>   of applying the WITH_THIS directive within a static function.   
>> This should always be
>>   correct.
>>
>>   This changeset also fixes a bug in that previously,  
>> ASTFunctionExpressions were not
>>   considered for the 'try/catch' treatment.  This effectively  
>> excluded state methods,
>>   and probably some other important cases.  Fixing this exposed an  
>> error in smoketest
>>   (only shown when catcherrors is true).
>>
>>       Tests: 880 Failures: 1 Errors: 1
>>       TestFailure: Â«<IntentionalErrors>Â»/testUndefinedErrors  
>> failed: Total number of expected errors:  expected 1 got 0
>>       TestError: Â«<IntentionalErrors>Â»/testUndefinedErrors  
>> failed: Wrong error:  expected Â«functionÂ» got 'testing: 1, 2, 3'
>>
>>   ptw believes this test case should be rewritten: That error is  
>> because that test is using arguments.callee to fake a closure  
>> (which is not going to work because the transform moves the code to  
>> an inner closure, and arguments.callee no longer gets it to the  
>> original function.
>>
>>
>> Tests:
>>   (dhtml,swf8,swf9) x (lzpix,weather,smokecheck,hello) x (enable  
>> catcherrors, disable catcherrors)
>>   -- smokecheck error for swf9 x smokecheck x (enable catcherrors)  
>> as noted above.
>>
>>
>> Files:
>> M      WEB-INF/lps/server/src/org/openlaszlo/sc/ 
>> JavascriptGenerator.java
>> M      WEB-INF/lps/server/src/org/openlaszlo/sc/CommonGenerator.java
>> M      WEB-INF/lps/server/src/org/openlaszlo/sc/CodeGenerator.java
>> M      WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/ 
>> ASTModifiedDefinition.java
>>
>> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20090111-dda-y.tar
>>
>>
>>
>> --
>>
>> Don Anderson
>> Java/C/C++, Berkeley DB, systems consultant
>>
>> voice: 617-306-2057
>> email: dda at ddanderson.com
>> www: http://www.ddanderson.com
>>
>>
>>
>>
>>
>

--

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

voice: 617-306-2057
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/20090111/f7fde387/attachment-0001.html