Comments:

1) Not your bug, but I suspect this is an error:

>           String baseName = name.substring(setterPrefix.length());
>           AttributeSpec superAttr = superclassModel.getAttribute(name, 
> ALLOCATION_INSTANCE);

Shouldn't that be `baseName` in the second line?  I think this is trying to 
check that if the compiler is defining $lzc$set_foo and the schema has <setter 
name='foo'> that `override` will be specified?

Otherwise approved.

On 2010-05-14, at 14:11, André Bargull wrote:

> Change 20100514-bargull-sfC by barg...@bargull02 on 2010-05-14 17:15:00
> in /home/anba/src/svn/openlaszlo/trunk
> for http://svn.openlaszlo.org/openlaszlo/trunk
> 
> Summary: fix compiler to handle overrides of final methods properly
> 
> Bugs Fixed: LPP-8982 (It's possible to "de-finalize" a method), LPP-8983 
> (Compiler fails to detect overriden final methods on instance classes), 
> LPP-8986 (Overriding final static method generates compiler warning)
> 
> Technical Reviewer: ptw
> QA Reviewer: max
> 
> Overview:
> Class-allocated methods aren't inherited to sub-classes, so any method 
> declaration check for overrides needs only to take place when defining 
> instance-allocated methods. This changes enforces this semantic and also 
> fixes some other bugs related to method overrides and final methods.
> 
> 
> Details:
> NodeModel:
> - removed some unused imports
> - added getUserTagName() to retrieve the tag-name used by the user, this is 
> important for instance classes to get the real tag-name instead of just 
> "anonymous"
> - addMethodInternal():
> + no override check required for class-allocated methods, because you cannot 
> override these methods
> + if an override was detected, but the user explicitely set override="false", 
> emit a warning
> + it's not yet possible to detect all overrides, so don't emit a warning if 
> the user explicitely set override="true", but no method declaration in a base 
> class was found
> + ViewSchema#checkInstanceMethodDeclaration() is only required for (1) 
> overriden, instance-allocated methods (2) class-allocated methods in <class> 
> definitions (or <mixin>, <interface>)
> + checkInstanceMethodDeclaration() needs the user tag-name, otherwise you'd 
> end up trying to get the attributes defined in a class named "anonymous"
> 
> ViewSchema:
> - removed some unused imports
> - checkMethodDeclaration():
> + only emit a warning when overriding an instance-allocated, final method
> - checkInstanceMethodDeclaration():
> + only check for methods whether an override is allowed
> + call isOverrideAllowed() instead of isMethodDeclarationAllowed(), otherwise 
> you'd skip a class (remember 'classname' is the name of the base class of the 
> current instance class, so if you search for super-class methods you need to 
> include 'classname')
> - addAttributeDefs():
> + only check overrides of instance-allocated attributes/methods
> 
> lfc-undeclared:
> - "apply" and "call" must not be redefined as class-allocated methods, but 
> it's okay to define both as instance-allocated methods
> 
> final-method-override-class-allocation:
> - test case for LPP-8986
> 
> final-method-override-instance-class:
> - test case for LPP-8983
> 
> final-method-override-reserved:
> - test case for schema changes
> 
> final-method-override:
> - test case for LPP-8982
> 
> invalid-user-method-override:
> - test case for detecting wrong override="false" from users
> 
> lztest-override-methods:
> - tests for various method override cases in lzx
> 
> lztest-override-methods-lzs:
> - tests for various method override cases in lzs
> 
> 
> Tests:
> smokecheck, amazon (swf8,swf10,dhtml)
> all testcases from this change set (swf8,swf10,dhtml)
> note: Some tests in test/compiler_errors will return compiler errors, this is 
> expected. Every file has got a section which explains the proper result.
> note: Some tests won't compile at all in swf10 (e.g. 
> final-method-override-class-allocation), but this is expected, too. Also see 
> LPP-8993.
> 
> Files:
> A test/compiler_errors/final-method-override-class-allocation.lzx
> A test/compiler_errors/final-method-override-instance-class.lzx
> A test/compiler_errors/final-method-override-reserved.lzx
> A test/compiler_errors/final-method-override.lzx
> A test/compiler_errors/invalid-user-method-override.lzx
> A test/lztest/lztest-override-methods.lzx
> A test/lztest/lztest-override-methods-lzs.lzx
> M WEB-INF/lps/schema/lfc-undeclared.lzx
> M WEB-INF/lps/server/src/org/openlaszlo/compiler/ViewSchema.java
> M WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java
> 
> Changeset: 
> http://svn.openlaszlo.org/openlaszlo/patches/20100514-bargull-sfC.tar
> 


Reply via email to