[Laszlo-checkins] r12420 - in openlaszlo/trunk/WEB-INF/lps/server: sc/src/org/openlaszlo/sc/parser src/org/openlaszlo/sc
dda@openlaszlo.org
dda at openlaszlo.org
Mon Jan 12 06:51:53 PST 2009
Author: dda
Date: 2009-01-12 06:51:48 -0800 (Mon, 12 Jan 2009)
New Revision: 12420
Modified:
openlaszlo/trunk/WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/ASTModifiedDefinition.java
openlaszlo/trunk/WEB-INF/lps/server/src/org/openlaszlo/sc/CodeGenerator.java
openlaszlo/trunk/WEB-INF/lps/server/src/org/openlaszlo/sc/CommonGenerator.java
openlaszlo/trunk/WEB-INF/lps/server/src/org/openlaszlo/sc/JavascriptGenerator.java
Log:
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: ?\194?\171<IntentionalErrors>?\194?\187/testUndefinedErrors failed: Total number of expected errors: expected 1 got 0
TestError: ?\194?\171<IntentionalErrors>?\194?\187/testUndefinedErrors failed: Wrong error: expected ?\194?\171function?\194?\187 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.
Modified: openlaszlo/trunk/WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/ASTModifiedDefinition.java
===================================================================
--- openlaszlo/trunk/WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/ASTModifiedDefinition.java 2009-01-12 09:00:08 UTC (rev 12419)
+++ openlaszlo/trunk/WEB-INF/lps/server/sc/src/org/openlaszlo/sc/parser/ASTModifiedDefinition.java 2009-01-12 14:51:48 UTC (rev 12420)
@@ -22,15 +22,32 @@
private Token t;
+ private void internalCopy(ASTModifiedDefinition that) {
+ this.access = that.access;
+ this.isStatic = that.isStatic;
+ this.isFinal = that.isFinal;
+ this.isDynamic = that.isDynamic;
+ this.isOverride = that.isOverride;
+ this.namespace = that.namespace;
+
+ this.id = that.id;
+ this.parser = that.parser;
+ this.filename = that.filename;
+ this.beginLine = that.beginLine;
+ this.beginColumn = that.beginColumn;
+ this.comment = that.comment;
+ this.t = that.t;
+ }
+
+ public SimpleNode shallowCopy() {
+ ASTModifiedDefinition result = new ASTModifiedDefinition(id);
+ result.internalCopy(this);
+ return result;
+ }
+
public SimpleNode deepCopy() {
ASTModifiedDefinition result = (ASTModifiedDefinition)super.deepCopy();
- result.access = this.access;
- result.isStatic = this.isStatic;
- result.isFinal = this.isFinal;
- result.isDynamic = this.isDynamic;
- result.isOverride = this.isOverride;
- result.namespace = this.namespace;
- result.t = this.t;
+ result.internalCopy(this);
return result;
}
@@ -152,6 +169,8 @@
verifyVariable(subnode);
else if (subnode instanceof ASTFunctionDeclaration)
verifyFunction(subnode);
+ else if (subnode instanceof ASTFunctionExpression)
+ verifyFunction(subnode);
else if (subnode instanceof ASTClassDefinition)
verifyClass(subnode);
// A nested function leaves behind an empty expression
@@ -166,6 +185,8 @@
verifyVariable(subnode);
else if (subnode instanceof ASTFunctionDeclaration)
verifyFunction(subnode);
+ else if (subnode instanceof ASTFunctionExpression)
+ verifyFunction(subnode);
else if (subnode instanceof ASTClassDefinition)
throw new ParseException("inner classes not allowed: " + subnode);
else
@@ -226,7 +247,7 @@
}
/* J_LZ_COPYRIGHT_BEGIN *******************************************************
-* Copyright 2007-2008 Laszlo Systems, Inc. All Rights Reserved. *
+* Copyright 2007-2009 Laszlo Systems, Inc. All Rights Reserved. *
* Use is subject to license terms. *
* J_LZ_COPYRIGHT_END *********************************************************/
Modified: openlaszlo/trunk/WEB-INF/lps/server/src/org/openlaszlo/sc/CodeGenerator.java
===================================================================
--- openlaszlo/trunk/WEB-INF/lps/server/src/org/openlaszlo/sc/CodeGenerator.java 2009-01-12 09:00:08 UTC (rev 12419)
+++ openlaszlo/trunk/WEB-INF/lps/server/src/org/openlaszlo/sc/CodeGenerator.java 2009-01-12 14:51:48 UTC (rev 12420)
@@ -1877,6 +1877,8 @@
LinkedHashMap fundefs = analyzer.fundefs;
Set closed = analyzer.closed;
Set free = analyzer.free;
+ boolean isStatic = isStatic(node);
+ boolean withThis = options.getBoolean(Compiler.WITH_THIS) && !isStatic;
// Note usage due to activation object and withThis
if (! free.isEmpty()) {
// TODO: [2005-06-29 ptw] with (_root) should not be
@@ -1885,7 +1887,7 @@
if (options.getBoolean(Compiler.ACTIVATION_OBJECT)) {
analyzer.incrementUsed("_root");
}
- if (options.getBoolean(Compiler.WITH_THIS)) {
+ if (withThis) {
analyzer.incrementUsed("this");
}
}
@@ -2108,7 +2110,7 @@
}
collector.emit(Instructions.WITH.make(block));
}
- if ((! free.isEmpty()) && options.getBoolean(Compiler.WITH_THIS)) {
+ if ((! free.isEmpty()) && withThis) {
if (registerMap.containsKey("this")) {
collector.push(Values.Register(((Instructions.Register)registerMap.get("this")).regno));
} else {
@@ -2657,6 +2659,6 @@
}
/* J_LZ_COPYRIGHT_BEGIN *******************************************************
-* Copyright 2001-2008 Laszlo Systems, Inc. All Rights Reserved. *
+* Copyright 2001-2009 Laszlo Systems, Inc. All Rights Reserved. *
* Use is subject to license terms. *
* J_LZ_COPYRIGHT_END *********************************************************/
Modified: openlaszlo/trunk/WEB-INF/lps/server/src/org/openlaszlo/sc/CommonGenerator.java
===================================================================
--- openlaszlo/trunk/WEB-INF/lps/server/src/org/openlaszlo/sc/CommonGenerator.java 2009-01-12 09:00:08 UTC (rev 12419)
+++ openlaszlo/trunk/WEB-INF/lps/server/src/org/openlaszlo/sc/CommonGenerator.java 2009-01-12 14:51:48 UTC (rev 12420)
@@ -605,7 +605,6 @@
// Scope #pragma directives to block
Compiler.OptionMap savedOptions = options;
try {
- options = options.copy();
for (int i = 0; i < dirs.length; i++) {
SimpleNode n = dirs[i];
List p = props;
@@ -614,6 +613,7 @@
ASTModifiedDefinition mod = null;
boolean ispublic = false;
boolean isconstructor = false;
+ options = savedOptions.copy();
if (n instanceof ASTModifiedDefinition) {
assert (n.getChildren().length == 1);
@@ -643,7 +643,17 @@
SimpleNode funexpr = new ASTFunctionExpression(0);
funexpr.setBeginLocation(n.filename, n.beginLine, n.beginColumn);
funexpr.setChildren(c);
- p.add(funexpr);
+
+ if (mod != null) {
+ mod = (ASTModifiedDefinition)mod.shallowCopy();
+ SimpleNode[] newchildren = new SimpleNode[1];
+ newchildren[0] = funexpr;
+ mod.setChildren(newchildren);
+ p.add(mod);
+ }
+ else {
+ p.add(funexpr);
+ }
} else if (how == TranslateHow.AS_INTERFACE) {
if (!isconstructor && ispublic) {
stmts.add(n);
@@ -890,6 +900,9 @@
return visitStatementList(node, children);
}
+ // An ASTModifiedDefinition can appear as a statement (modifying
+ // class and function definitions) or as an expression (modifying
+ // function expressions)
public SimpleNode visitModifiedDefinition(SimpleNode node, SimpleNode[] children) {
// Modifiers, like 'final', are ignored unless this is handled
// by the runtime.
@@ -902,6 +915,18 @@
return visitStatement(child);
}
+ public SimpleNode visitModifiedDefinitionExpression(SimpleNode node, boolean isReferenced, SimpleNode[] children) {
+ // Modifiers, like 'final', are ignored unless this is handled
+ // by the runtime.
+
+ assert children.length == 1;
+ SimpleNode child = children[0];
+
+ ((ASTModifiedDefinition)node).verifyTopLevel(child);
+
+ return visitExpression(child, isReferenced);
+ }
+
public SimpleNode visitLabeledStatement(SimpleNode node, SimpleNode[] children) {
ASTIdentifier name = (ASTIdentifier)children[0];
SimpleNode stmt = children[1];
@@ -1124,6 +1149,9 @@
else if (node instanceof ASTAssignmentExpression) {
newNode = visitAssignmentExpression(node, isReferenced, children);
}
+ else if (node instanceof ASTModifiedDefinition) {
+ newNode = visitModifiedDefinitionExpression(node, isReferenced, children);
+ }
else if (node instanceof Compiler.PassThroughNode) {
newNode = node;
}
@@ -1186,9 +1214,15 @@
}
return source;
}
+
+ boolean isStatic(SimpleNode node) {
+ SimpleNode parent = node.getParent();
+ return (parent instanceof ASTModifiedDefinition &&
+ ((ASTModifiedDefinition)parent).isStatic());
+ }
}
/* J_LZ_COPYRIGHT_BEGIN *******************************************************
-* Copyright 2001-2008 Laszlo Systems, Inc. All Rights Reserved. *
+* Copyright 2001-2009 Laszlo Systems, Inc. All Rights Reserved. *
* Use is subject to license terms. *
* J_LZ_COPYRIGHT_END *********************************************************/
Modified: openlaszlo/trunk/WEB-INF/lps/server/src/org/openlaszlo/sc/JavascriptGenerator.java
===================================================================
--- openlaszlo/trunk/WEB-INF/lps/server/src/org/openlaszlo/sc/JavascriptGenerator.java 2009-01-12 09:00:08 UTC (rev 12419)
+++ openlaszlo/trunk/WEB-INF/lps/server/src/org/openlaszlo/sc/JavascriptGenerator.java 2009-01-12 14:51:48 UTC (rev 12420)
@@ -1250,16 +1250,16 @@
// Having another function closure within a closure apparently
// causes problems accessing certain variables for the flex compiler.
boolean tryAll = options.getBoolean(Compiler.CATCH_FUNCTION_EXCEPTIONS)
- && matchingAncestor(node.getParent(), ASTFunctionExpression.class) == null
- && matchingDescendant(node, ASTSuperCallExpression.class) == null
- && matchingDescendant(node, ASTFunctionExpression.class) == null
+ && matchingAncestor(node, ASTFunctionExpression.class, false) == null
+ && matchingDescendant(node, ASTSuperCallExpression.class, false) == null
+ && matchingDescendant(node, ASTFunctionExpression.class, false) == null
&& functionName != null;
String tryType = "";
if (tryAll) {
error.add(parseFragment("if ($debug) {" +
" $lzsc$runtime.reportException(" +
- ScriptCompiler.quote(functionNameIdentifier.filename) + ", " +
+ ScriptCompiler.quote(filename) + ", " +
functionNameIdentifier.beginLine + ", $lzsc$e); }"));
predecls.add(new Compiler.PassThroughNode(parseFragment("var $lzsc$ret:* = 0;")));
@@ -1298,6 +1298,8 @@
LinkedHashMap fundefs = analyzer.fundefs;
Set closed = analyzer.closed;
Set free = analyzer.free;
+ boolean isStatic = isStatic(node);
+ boolean withThis = options.getBoolean(Compiler.WITH_THIS) && !isStatic;
// Note usage due to activation object and withThis
if (! free.isEmpty()) {
// TODO: [2005-06-29 ptw] with (_root) should not be
@@ -1306,7 +1308,7 @@
if (options.getBoolean(Compiler.ACTIVATION_OBJECT)) {
analyzer.incrementUsed("_root");
}
- if (options.getBoolean(Compiler.WITH_THIS)) {
+ if (withThis) {
analyzer.incrementUsed("this");
}
}
@@ -1460,7 +1462,7 @@
}
// If the locals are not remapped, we assume we are in a runtime
// that already does implicit this in methods...
- if ((! free.isEmpty()) && options.getBoolean(Compiler.WITH_THIS) && remapLocals()) {
+ if ((! free.isEmpty()) && withThis && remapLocals()) {
SimpleNode newStmts = new ASTStatementList(0);
newStmts.setChildren((SimpleNode[])stmtList.toArray(new SimpleNode[0]));
SimpleNode withNode = new ASTWithStatement(0);
@@ -1483,7 +1485,13 @@
Map map = new HashMap();
newStmts = visitStatement(newStmts);
map.put("_1", newStmts);
- String frag = "$lzsc$ret = (function()" + tryType + " { with (this) { _1 }}).call(this);";
+ String frag = "$lzsc$ret = (function()" + tryType + " {";
+ if (isStatic) {
+ frag += " { _1 }}).call(null);";
+ }
+ else {
+ frag += " with (this) { _1 }}).call(this);";
+ }
newStmts = new Compiler.PassThroughNode((new Compiler.Parser()).substitute(newStmts, frag, map));
}
tryNode.set(i++, newStmts);
@@ -1555,29 +1563,30 @@
return new SimpleNode[] { node, null };
}
- // walk up the AST and find a match by type
- SimpleNode matchingAncestor(SimpleNode node, Class matchClass) {
+ // walk up the AST and find a match by type, optionally skipping the original node
+ SimpleNode matchingAncestor(SimpleNode node, Class matchClass, boolean includeThis) {
while (node != null) {
- if (matchClass.equals(node.getClass())) {
+ if (includeThis && matchClass.equals(node.getClass())) {
return node;
}
+ includeThis = false;
node = node.getParent();
}
return null;
}
- // walk down the AST and find a match by type
- SimpleNode matchingDescendant(SimpleNode node, Class matchClass) {
+ // walk down the AST and find a match by type, optionally skipping the original node
+ SimpleNode matchingDescendant(SimpleNode node, Class matchClass, boolean includeThis) {
if (node == null) {
return null;
}
- else if (matchClass.equals(node.getClass())) {
+ else if (matchClass.equals(node.getClass()) && includeThis) {
return node;
}
else {
SimpleNode[] children = node.getChildren();
for (int i=0; i<children.length; i++) {
- SimpleNode result = matchingDescendant(children[i], matchClass);
+ SimpleNode result = matchingDescendant(children[i], matchClass, true);
if (result != null) {
return result;
}
@@ -1846,7 +1855,7 @@
}
/**
- * @copyright Copyright 2006-2008 Laszlo Systems, Inc. All Rights
+ * @copyright Copyright 2006-2009 Laszlo Systems, Inc. All Rights
* Reserved. Use is subject to license terms.
*/
More information about the Laszlo-checkins
mailing list