If you compile warnWithThis, we'll see why.  Here's a guess, that the mixins 
are not really handled:

  public dynamic class LzDataset extends LzNode with LzDataElementMixin, 
LzDataNodeMixin

I think the straight inheritance chain may be generated as reference class, but 
not the mixins.  Currently, the schema does not know about mixin inheritance.  
Solutions: 
  - Add mixins to the schema, and generate reference classes for any needed 
mixins.
  - Generate reference classes for everything (but I know this had some issues 
when I tried it).

But failures like that are detected, and the fallback of generating with (this) 
should work.  99% coverage is not bad!  Like to hear what performance diff 
there would be.

- Don

On Mar 9, 2011, at 1:29 PM, P T Withington wrote:

> I verified that I can compile webtop with your change and eliminate nearly 
> all uses of 'with (this)':
> 
> main.lzx.don:66
> main.lzx.js:6569
> 
> A spot-check reveals most of the remaining uses of 'with (this)' are 
> apparently due to the compiler having incomplete information for lz.dataset?  
> Could that be?
> 
> On 2011-03-03, at 10:46, Donald Anderson wrote:
> 
>> (Re-review with fixes for Andre's test cases):
>> 
>> Change dda-20110301-0dx by [email protected] on 2011-03-01 14:23:55 EST
>>   in /Users/dda/laszlo/src/svn/openlaszlo/trunk-c/WEB-INF/lps/schema
>>   for http://svn.openlaszlo.org/openlaszlo/trunk/WEB-INF/lps/schema
>> 
>> Summary: Eliminate the need for with(this) from most typical cases
>> 
>> New Features:
>> 
>> Bugs Fixed: LPP-8751 Eliminate the use of `with(this)`
>> 
>> Technical Reviewer: ptw (pending)
>> QA Reviewer: henry (pending)
>> Doc Reviewer: (pending)
>> 
>> Documentation:
>> 
>> Release Notes:
>> 
>> Overview:
>>   with(this) can be eliminated from a function only when either of these
>>   conditions occur:
>>    - we have an complete knowledge of the class heirarchy.
>>    - there are no free references in the function that cannot be
>>      accounted for as globals.
>>   and when this condition does not occur:
>>    - another 'with(varname)' appears in the function.
>>   We address the first condition via changes to the js2doc as it
>>   generates the lfc schema, the tag compiler as it generates extra
>>   'reference' JS classes that contain attributes and methods with
>>   empty implementations, and the script compiler, as it uses the
>>   reference classes to create a (usually) complete picture of the class
>>   heirarchy for most cases.  The second condition is addressed via
>>   a prescan of the AST for globals and class names.  As for the third
>>   condition, there are ways of mitigating it, but we don't bother for now
>>   on the assumption that a function coded containing 'with(varname)' is not
>>   a function that is coded for speed.
>> 
>>   There may be some future work that may require a complete set of
>>   reference classes, or method args, etc..  These are mentioned in
>>   comments below as 'FUTURE WORK'.  For most of these, preliminary
>>   explorations were done, and abandoned for expediency.
>> 
>> Details:
>>   [Four files changed since 3/1 review sent:
>>    CommonGenerator.java, JavascriptGenerator.java,
>>    WithThisAnalyzer.java, lztest-class-impl.lzx]
>> 
>>   test/lztest/lztest-class-impl.lzx:
>>     Added some simple tests that illustrate how with(this) is inserted
>>     in certain cases (before this change).  By inspecting JS, used to
>>     verify that 'this.' insertion and 'with(this)' removal is working.
>> 
>>   WEB-INF/lps/schema/lfc-undeclared.lzx:
>>     Added attribute: isinited, marked as private in doc, which is used
>>     by old components, and cannot be marked as public.
>>     Added method: setAttribute, which actually lives in LzEventable, and is
>>     used quite often.  We don't current emit reference classes for
>>     any classes that are not associated with a tag (it was too hard
>>     to get a non-tag class into the tag compiler - FUTURE WORK).
>> 
>>   WEB-INF/lps/lfc/core/LzNode.lzs:
>>     Added devnote about isinited.
>> 
>>   WEB-INF/lps/lfc/compiler/Class.lzs:
>>     Minor error message improvement that I found helpful once. (optional)
>> 
>>   WEB-INF/lps/server/src/org/openlaszlo/js2doc/SchemaBuilder.java:
>>     - jsname (the javascript name) for a class as well as jsextends
>>       (the javascript class it extends) are put into the schema, though
>>       not currently used - FUTURE WORK).
>>     - omit event names not already defined as attributes
>>     - method parameters are now collected and inserted into the schema,
>>       but no typing is currently preserved (FUTURE WORK).
>> 
>>   WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassCompiler.java:
>>     Ignore classes in the schema that has no associated tag name (FUTURE 
>> WORK).
>>     Small change to the protocol of compiling a ClassModel.
>> 
>>   WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java:
>>     Ignore the jsname, jsextends parts of the schema (FUTURE WORK).
>>     Small change to the protocol of compiling a ClassModel.
>> 
>>   WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassModel.java:
>>     - Changed isCompiled() to be true to its name.
>>     - reworked compile() to handle emiting reference classes as needed.
>>     - reworked emitClassDeclaration() to work with reference classes.
>> 
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/ScriptClass.java:
>>     Changed generation of a class so it won't create attributes named 'with'
>>     classes named 'class', etc.  Needed when generating all classes for
>>     the full schema (possibly optional now, but needed for FUTURE WORK).
>> 
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/CommonGenerator.java:
>>     - rearrange globals and debugGlobals into separate sets, enumerate
>>       the members of JS 'Object' class.
>>     - ClassDescriptor.getInstanceProperties() modified to return an 
>> 'incomplete' set,
>>       rather than null.  ClassDescriptor.complete is set and needs to be 
>> checked
>>       by the caller.
>>     - ClassDescriptor.incompleteSet() added for informational messages.
>>     - ClassDescriptor.toString() for development/debugging.
>>     - process reference classes minimally when needed at all.
>> 
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/JavascriptGenerator.java:
>>     - create set of forwardGlobals by doing a shallow walk of the AST.
>>     - manage the 'possibleInstance' set to try to reduce it and move
>>       as many items as appropriate/possible to the 'bindthis' set,
>>       which is the set of variables we will actively bind to 'this'.
>>     - issue warning if 'warnWithThis' is on.
>> 
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/SWF9Generator.java:
>>     Make sure reference classes are ignored in this generator.
>> 
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/DHTMLCompiler.java:
>>     set option so that reference classes are accepted.
>> 
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/Compiler.java:
>>     define 'referenceClass' pragma issued by tag compiler.
>>     define 'registerReferenceClasses', an option only set by the 
>> DHTMLCompiler.
>>     define 'warnWithThis', a new debugging option that will warn
>>     when with(this) is used, and give the reason behind it, e.g.:
>>        'with(this) added in foobar: unknown parts of class hierarchy: 
>> [LzTestManagerClass->LzEventable] and unaccounted refs: [LzHTTPLoader, 
>> escape]'
>> 
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/CodeGenerator.java:
>>     Abide by new ClassDescriptor protocol
>> 
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/GenericVariableAnalyzer.java:
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/VariableAnalyzer.java:
>>   WEB-INF/lps/server/src/org/openlaszlo/sc/WithThisAnalyzer.java:
>>     Refactored VariableAnalyzer so essential parts could be shared
>>     with a new 'WithThisAnalyzer'.  The new analyzer walks the tree
>>     and applies identifier --> this.identifier transformation for
>>     any free variables found in a set.  It also tracks if any
>>     'with' constructs are found, which is a spoiler for with(this) removal.
>> 
>>   WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/SimpleNode.java:
>>     Added indexOf() - a convenience function, helpful in the 'this binding'.
>> 
>> Tests:
>> - Ran smoketest (DHTML,SWF10) lztest, runlzunit
>> - after lztest, examined *.js files left behind to determine that
>>   79% (286 of 362) of occurrences of with(this) had been removed compared
>>   with a run without these changes.  Spot checked to ensure that 'this.'
>>   was inserted, specifically in the 'WithThis' tests
>>   (in test/lztest/lztest-class-impl.lzx)
>> -  Of the occurrences left, we know why they occur: 1) LzTestManagerClass 
>> inherits
>>   from LzEventable, not a class associated with a tag.  This will not happen 
>> for
>>   pure lzx programs. 2) they often refer to use class names in the LFC:
>>   LzHTTPLoader, LzTimeKernel.  3) they refer to identifiers 'canvas', 
>> 'escape'.
>> 
>> 
>> 
>> Files:
>> M       test/lztest/lztest-class-impl.lzx
>> M       WEB-INF/lps/schema/lfc-undeclared.lzx
>> M       WEB-INF/lps/lfc/core/LzNode.lzs
>> M       WEB-INF/lps/lfc/compiler/Class.lzs
>> M       WEB-INF/lps/server/src/org/openlaszlo/js2doc/SchemaBuilder.java
>> A       WEB-INF/lps/server/src/org/openlaszlo/sc/WithThisAnalyzer.java
>> M       WEB-INF/lps/server/src/org/openlaszlo/sc/JavascriptGenerator.java
>> A       WEB-INF/lps/server/src/org/openlaszlo/sc/GenericVariableAnalyzer.java
>> M       WEB-INF/lps/server/src/org/openlaszlo/sc/VariableAnalyzer.java
>> M       WEB-INF/lps/server/src/org/openlaszlo/sc/SWF9Generator.java
>> M       WEB-INF/lps/server/src/org/openlaszlo/sc/CommonGenerator.java
>> M       WEB-INF/lps/server/src/org/openlaszlo/sc/DHTMLCompiler.java
>> M       WEB-INF/lps/server/src/org/openlaszlo/sc/Compiler.java
>> M       WEB-INF/lps/server/src/org/openlaszlo/sc/CodeGenerator.java
>> M       WEB-INF/lps/server/src/org/openlaszlo/sc/ScriptClass.java
>> M       WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassCompiler.java
>> M       WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java
>> M       WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassModel.java
>> M       WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/SimpleNode.java
>> 
>> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/dda-20110301-0dx.tar
>> 
>> 
>> 
>> --
>> 
>> Don Anderson
>> Java/C/C++, Berkeley DB, systems consultant
>> 
>> voice: 617-306-2057
>> email: [email protected]
>> www: http://www.ddanderson.com
>> blog: http://libdb.wordpress.com
>> 
>> 
>> 
>> 
>> 
> 


--

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

voice: 617-306-2057
email: [email protected]
www: http://www.ddanderson.com
blog: http://libdb.wordpress.com





Reply via email to