Change 20070921-dda-l by [EMAIL PROTECTED] on 2007-09-21 17:52:38 EDT
in /Users/dda/laszlo/src/svn/openlaszlo/trunk
for http://svn.openlaszlo.org/openlaszlo/trunk
Summary: Fix script compiler switch stack mixup in default label
New Features:
Bugs Fixed: LPP-3023
Technical Reviewer: ptw
QA Reviewer: (pending)
Doc Reviewer: (pending)
Documentation:
Release Notes:
Details:
The problem is in the script compiler, specifically for the SW7/8
code generator.
When creating a jump table (a series of comparisons to jump to
the case labels),
each jump action leaves a value at the top of the stack that must
be popped as the
first action of the case label. However, default was handled
differently.
The extra value was popped first and the jump to the default
label is made,
so the default label did not pop as its first action.
Here's the problem. When case labels are joined with the default
label like this:
case 10:
case 11:
default:
// do something...
the code generator attempts to share the code, but the stack
levels are mismatched.
One solution is to force the default label to be the last and
insert the extra pop
for the case labels only:
case 10:
case 11:
pop stack
// fall through
default:
I opted for a simpler solution, to guarantee that stack levels
are always the same.
During the jump table, we avoid popping the stack before the
default jump. We must
take care that if there is no default label, the stack is popped
before the jump table
completes and we jump to the end of the switch code block.
With this approach, we now always emit a pop as the first action
of a default
label, like every other case label. This allows case labels to
be easily
joined whether or not they represent the default case.
I don't believe there is any performance impact: we are doing the
same number of pops
at runtime (well, one extra that is needed to fix the buggy
situation), and the
overall code size should be the same.
Tests:
Ran test listed in LPP-3023 and made sure it is now correct, both
with debug and no debug.
Enhanced the app to test a number of other corner cases for
default labels and verified
the output, again with/without debug. App included here:
<canvas>
<class name="switcher" extends="text">
<method name="testSwitch" args="val">
var result = "none";
switch (val) {
case 1:
result = "case 1";
break;
case 2:
case 3:
// if you uncomment the true below both 2 and 3 work
// true;
default:
// if you remove the default: above 2 and 3 also work
result = "default";
break;
}
this.setText(this.name + ": " + result);
</method>
<method name="testSwitchDefaultBreak" args="val">
var result = "none";
switch (val) {
case 1:
result = "case 1";
break;
case 2:
case 3:
// if you uncomment the true below both 2 and 3 work
// true;
default:
break;
}
this.setText(this.name + ": " + result);
</method>
<method name="testSwitchDefaultNoBreak" args="val">
var result = "none";
switch (val) {
case 1:
result = "case 1";
break;
case 2:
case 3:
// if you uncomment the true below both 2 and 3 work
// true;
default:
}
this.setText(this.name + ": " + result);
</method>
<method name="testSwitchNoDefault" args="val">
var result = "none";
switch (val) {
case 1:
result = "case 1";
break;
case 2:
case 3:
// if you uncomment the true below both 2 and 3 work
// true;
}
this.setText(this.name + ": " + result);
</method>
</class>
<simplelayout />
<switcher name="switch1" />
<switcher name="switch2" />
<switcher name="switch3" />
<switcher name="switch4" />
<switcher name="switch11" />
<switcher name="switch21" />
<switcher name="switch31" />
<switcher name="switch41" />
<switcher name="switch12" />
<switcher name="switch22" />
<switcher name="switch32" />
<switcher name="switch42" />
<switcher name="switch13" />
<switcher name="switch23" />
<switcher name="switch33" />
<switcher name="switch43" />
<text name="switch5">
<method name="otherSwitch" args="val">
var result = "none";
switch (val) {
case 1:
result = "case 1";
break;
case 2:
case 3:
// if you uncomment the true below both 2 and 3 work
// true;
default:
// if you remove the default: above 2 and 3 also work
result = "default";
break;
}
this.setText(this.name + ": " + result);
</method>
</text>
<text name="switch6">
<method name="nonSwitch" args="val">
var result = "none";
if (val == 1) {
result = "case 1";
} else {
result = "default";
}
this.setText(this.name + ": " + result);
</method>
</text>
<text name="switch7">
</text>
<script>
// the first case works
switch1.testSwitch(1);
// the default case works;
switch2.testSwitch(5);
// using 2 or 3 appears to work but breaks all method calls
switch3.testSwitch(2);
// none of these locally defined calls work
switch4.testSwitch(1);
switch11.testSwitchDefaultBreak(1);
switch21.testSwitchDefaultBreak(5);
switch31.testSwitchDefaultBreak(2);
switch41.testSwitchDefaultBreak(1);
switch12.testSwitchDefaultNoBreak(1);
switch22.testSwitchDefaultNoBreak(5);
switch32.testSwitchDefaultNoBreak(2);
switch42.testSwitchDefaultNoBreak(1);
switch13.testSwitchNoDefault(1);
switch23.testSwitchNoDefault(5);
switch33.testSwitchNoDefault(2);
switch43.testSwitchNoDefault(1);
switch5.otherSwitch(1);
switch6.nonSwitch(1);
switch7.setText(this.name + ": " + "test");
</script>
</canvas>
Files:
M WEB-INF/lps/server/src/org/openlaszlo/sc/CodeGenerator.java
Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20070921-dda-
l.tar
--
Don Anderson
Java/C/C++, Berkeley DB, systems consultant
Voice: 617-547-7881
Email: [EMAIL PROTECTED]
WWW: http://www.ddanderson.com
