[Laszlo-dev] For Review: Change 20090111-dda-y Summary: Remove references to 'this' in static functions (revised 2)
Donald Anderson
dda at ddanderson.com
Mon Jan 12 07:05:12 PST 2009
Improvement request filed here:
http://www.openlaszlo.org/jira/browse/LPP-7617
Assigned to me 4.2.1 P1
On Jan 12, 2009, at 9:16 AM, P T Withington wrote:
> Can you check in what you have then (ignoring my suggestion).
>
> Let's get a solution for the immediate problem and open a new
> 'improvement' with these suggestions.
>
> On 2009-01-12, at 00:36EST, Donald Anderson wrote:
>
>>
>> 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
>>
>>
>>
>>
>>
>
--
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/20090112/2d7ae46c/attachment-0001.html
More information about the Laszlo-dev
mailing list