[Laszlo-dev] For Review: Change 20081021-dda-p Summary: Add return type declarations to SWF9

Donald Anderson dda at ddanderson.com
Tue Oct 21 10:57:28 PDT 2008 

On Oct 21, 2008, at 1:01 PM, P T Withington wrote:

> Great stuff!
>
> Comments:
>
> 1) Our idiom for undefined is `void 0`, because of (http://tinyurl.com/2z3q2o 
> ),

On an existing line with 'return void(0);'  , I got a flex error about  
mismatching type.
I think I changed this to return LzReplicationManager(0) but I got a  
test failure.  Quite possibly
it was in DHTML only, so maybe '0 cast LzReplicationManager' would fix  
it.
(My fix was 'return this' - it got past the compiler and seemed to  
pass the errant test).

> OTOH, in the case of return, shouldn't you be able to say simply  
> `return ;` to return undefined, since `;` is an empty expression  
> whose value is undefined.  Is it us or Flex that says that is an  
> error?

Flex error.

> OTOOH, in the case of returning undefined for a typed method, is  
> flex just coercing undefined to the type's null?  E.g.:
>
>  lzx> (function ():Boolean { return void 0; })()
>  false
>
> Says to me you might as well just return `false`.

In (at least one) case of Boolean , I did have to return false to get  
tests to work.  Given this, I will change all Boolean
functions to return false wherever I put in undefined before.

>  OTOOOH, Flex does not seem to coerce undefined to null for subtypes  
> of Object.  Curious.
>
> 2) toString has a much larger problem, since it is intending to  
> override the Object.toString method, but will not unless it is  
> marked public, and if it is, it will then need to match the  
> signature of the base method.  Please file a separate bug to repair  
> toString, if one does not already exist.

Alright

> 3) André's point:  don't use classes as functions to cast.  It's not  
> reliable.

Yes, I'll fix that.

> 4) In LzReplicationManager, if we can't figure out what is going on,  
> please file a bug.  It looks to me like the type declaration is too  
> narrow, that it should be LzNode not LzView.  (Pet peeve of mine:   
> the frickin' compiler knows the type I should be declaring, why  
> should I have to tell it?)

Okay, I'll file a JIRA

>
> 5) I don't understand the change in the return value of  
> __LZHandleMultiNodes.  This seems like a semantic change that needs  
> a separate bug?

Okay, I'll file a JIRA


> 6) Another point by André: For undefined Number results it would be  
> more accurate to return NaN.
>

Yes, I'll fix that.

> 7) swf9/LzDebug remove the @devnote now that you have addressed the  
> bug

Okay.

> Otherwise, approved.
>

Also, since I needed to change lzpix application code to add a return  
type for an overridden method,
I think we'll need to make a run-through all the sample apps before  
the release goes out.
Though that seems to be the only occurrence of isMultiSelect() in the  
demos directory.

Thanks,

- Don

> On 2008-10-21, at 10:54EDT, Donald Anderson wrote:
>
>> Change 20081021-dda-p by dda at lester.local on 2008-10-21 10:03:53 EDT
>>   in /Users/dda/laszlo/src/svn/openlaszlo/trunk-a
>>   for http://svn.openlaszlo.org/openlaszlo/trunk
>>
>> Summary: Add return type declarations to SWF9
>>
>> New Features:
>>
>> Bugs Fixed: LPP-7031 (swf9 back-end is dropping return type  
>> declarations)
>>
>> Technical Reviewer: ptw (pending)
>> QA Reviewer: promanik (pending)
>> Doc Reviewer: (pending)
>>
>> Documentation:
>>
>> Release Notes:
>>
>> Details:
>>
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/ParseTreePrinter.java
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/SWF9ParseTreePrinter.java
>>
>>     Explicit return types like Boolean in 'function foo () :  
>> Boolean { }' were
>>     not being emitted by the SWF9 backend.  The fix was  
>> straightforward,
>>     the fallout was not...
>>
>>
>>   WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/Method.java
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/Function.java
>>
>>     Added a "type" attribute to <method> to set the return type.
>>     This was needed to get a few cases where lzx code overrides LFC  
>> functions (now with return typing!)
>>     to compile.  This type must be 'Javascript' (i.e. Boolean, not  
>> 'boolean' is used).
>>     Some cleanup to reduce the number of constructors for Method/ 
>> Function.  I think
>>     this makes the code a bit clearer.
>>
>>   lps/components/base/datalistselector.lzx
>>   lps/components/base/listselector.lzx
>>   demos/lzpix/app.lzx
>>
>>     Added type="Boolean" to a few methods that override to allow  
>> them to compile and
>>     run in SWF9.
>>
>>
>>   Fixing return typing exposed a lot of 'new' typing in LFC and  
>> some in application code,
>>   which caused various consistency errors in several categories.
>>
>>   - in many .lzs files, functions marked as returning a type did  
>> not always
>>     explicitly call return, or when they did they did not return a  
>> value.
>>     Initially, I tried to make intelligent guesses about values  
>> returned (favoring 'null'),
>>     but testing revealed many problems.  Ultimately, I needed to be
>>     very conservative, returning 'undefined' in most cases, and false
>>     for Boolean functions.  This seems to be the default behavior  
>> for JS
>>     when returns are missing values.  Cases that seemed most  
>> questionable
>>     I marked with:
>>         TODO: [20081020 dda] review this return value change from  
>> LPP-7031
>>
>>
>>   - in several .lzs files, functions marked as override did not  
>> always match return types.
>>
>>
>>   - in several .lzs files, toString() is marked as :String, but in  
>> others it is not.
>>     For the moment, it was too big of a job to fix all these as  
>> part of this (already
>>     large) commit, so typing for this function was commented  
>> whenever a
>>     compile error showed a mismatch.
>>
>>
>>   - in several .lzs files, functions with new typing returned a  
>> value but without proper typing.
>>     Type casting was added as appropriate.
>>
>>
>>   WEB-INF/lps/lfc/kernel/swf9/LzMouseKernel.as:
>>
>>   - type of a local variable wasn't narrowly typed enough, leading to
>>     errors in return statements.
>>
>>
>>   WEB-INF/lps/lfc/debugger/LzDebug.lzs:
>>
>>   - __StringDescription() needed to be defined with exactly the  
>> same argument signature
>>     I don't know why return typing exposed this as a problem.
>>
>>
>>   WEB-INF/lps/lfc/data/LzLazyReplicationManager.lzs:
>>
>>   - Certain local variables typed as LzView needed typing  
>> commented.  Initial attempts
>>     to fix the typing errors in this file involved casting, e.g.
>>            var v:LzView = LzView(this.getNewClone( true ));
>>     But these changes broke smokecheck for DHTML.  It seems that  
>> some assumption
>>     about typing is being violated and will need to be tracked down  
>> later.
>>     I commented out the typing for the local variables, and left  
>> TODOs to clean
>>     this up later.
>>
>>
>>  Note that the compile errors fixed here only show up when building  
>> the LFC (or app) for SWF9.
>>  In debugging these problems it was most helpful to build the  
>> debuglfc to get line number information.
>>
>> Tests:
>>  tests: (smokecheck) * (swf8+dhtml)
>>  apps: (weather+lzpix) * (swf8+swf9+dhtml), hello(swf9)
>>
>> Files:
>> M      WEB-INF/lps/lfc/kernel/swf9/LFCApplication.as
>> M      WEB-INF/lps/lfc/kernel/swf9/LzMouseKernel.as
>> M      WEB-INF/lps/lfc/services/LzAudio.lzs
>> M      WEB-INF/lps/lfc/debugger/LzDebug.lzs
>> M      WEB-INF/lps/lfc/debugger/platform/swf9/LzFlashRemote.as
>> M      WEB-INF/lps/lfc/debugger/platform/swf9/LzDebug.as
>> M      WEB-INF/lps/lfc/views/LaszloView.lzs
>> M      WEB-INF/lps/lfc/controllers/LzAnimatorGroup.lzs
>> M      WEB-INF/lps/lfc/controllers/LaszloAnimation.lzs
>> M      WEB-INF/lps/lfc/data/LzLazyReplicationManager.lzs
>> M      WEB-INF/lps/lfc/data/LzReplicationManager.lzs
>> M      WEB-INF/lps/lfc/data/LzDatapointer.lzs
>> M      WEB-INF/lps/lfc/data/LzDataText.lzs
>> M      WEB-INF/lps/lfc/data/LzDataNode.lzs
>> M      WEB-INF/lps/lfc/data/LzDataElement.lzs
>> M      WEB-INF/lps/lfc/data/LzDatapath.lzs
>> M      WEB-INF/lps/lfc/data/LzResizeReplicationManager.lzs
>> M      WEB-INF/lps/lfc/data/LzParam.lzs
>> M      WEB-INF/lps/lfc/data/LzDataAttrBind.lzs
>> M      WEB-INF/lps/lfc/compiler/LzBootstrapDebugService.lzs
>> M      WEB-INF/lps/server/src/org/openlaszlo/sc/Method.java
>> M      WEB-INF/lps/server/src/org/openlaszlo/sc/ 
>> SWF9ParseTreePrinter.java
>> M      WEB-INF/lps/server/src/org/openlaszlo/sc/Function.java
>> M      WEB-INF/lps/server/src/org/openlaszlo/sc/ParseTreePrinter.java
>> M      WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java
>> M      lps/components/base/datalistselector.lzx
>> M      lps/components/base/listselector.lzx
>> M      demos/lzpix/app.lzx
>>
>> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20081021-dda-p.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/20081021/245e6035/attachment-0001.html

More information about the Laszlo-dev mailing list