Logical Or Assignment Bug in ASC2

So, here’s something to keep an eye on if switching to AIR 3.8 (or 3.7 with the ASC2 compiler).

Last week at work we decided to finally make the jump from AIR 3.5 to AIR 3.8; something I was pretty excited about and had been looking forward to for awhile. After applying the update, though, I noticed some unit tests failing and sat down to investigate. Several places in our app, we map loaded xml files to data objects – a pretty common thing to do. In the parse methods we may use typical logical or assignment to fill properties with default values if they aren’t present in the xml. So it’s not uncommon to run across a line like this:

this.name = xml.@name || “Default Name”;

Now, in the ASC1 compiler this always worked fine. After switching to the ASC2 compiler in AIR 3.8, though, this is where our unit tests began falling down. In the case above, rather than assigning the “Default Name”, the name property was being assigned an empty string. So I decided to have a little look under the hood and see exactly what was going on. So here’s a real quick test class file I compiled to examine closer:

package 
{

import flash.display.Sprite;
import flash.events.Event;

public class Main extends Sprite 
{

	public function Main():void 
	{
		if (stage) init();
		else addEventListener(Event.ADDED_TO_STAGE, init);
	}

	private function init(event:Event = null):void 
	{
		removeEventListener(Event.ADDED_TO_STAGE, init);

		testXML();
	}

	private function testXML():void
	{
		var xml:XML = 

		var name:String = xml.@name || "Default";

		trace("Name=(" + name + ")");
		// output:
		// ASC1: Name=(Default)
		// ASC2: Name=()

	}
}

}

As you can see in the comments, compiling with ASC1, I got the results I was looking for and used to. In ASC2 though, I got an empty string. Using Adobe’s SWF Investigator, I decompiled the two swf files to the Actionscript Bytecode (ABC). With the ASC1, here is what’s going on at the point you try to get the xml’s name attribute:

32   getproperty   	name //nameIndex = 10
34   coerce_s      	
35   dup           	
36   convert_b     	
37   iftrue        	L1

41   pop           	
42   pushstring    	"Default"  //stringIndex = 27
44   coerce_s      	

L1: 
45   coerce_s      	
46   setlocal2

Now, I’m no expert in ABC, but it’s easy enough to follow along with what’s happening. We get the property name from the xml and type it to a String (‘coerce_s’). Since there is no property ‘name’, that will be coerced to an empty String. We then convert that to a Boolean (‘convert_b’). Of course an empty string converted to a boolean is false so the ‘iftrue’ fails, we push the string “Default” on to the stack, type it as a string (‘coerce_s’), then set the local variable ‘name’ to that value.

Compiled in ASC2 though, we see a different story:

28   getproperty   	name //nameIndex = 9
30   dup           	
31   iftrue        	L1

35   pop           	
36   pushstring    	"Default"  //stringIndex = 31
38   coerce_a      	

L1: 
39   coerce_s      	
40   setlocal1

This time, we get the property name but do no coercion or conversion – we just wind up with an an untyped ‘something’. But an untyped ‘something’ is not a ‘nothing’, so the iftrue passes, we jump to the L1 block where we then coerce that ‘something’ into an empty string and set set the local variable ‘name’ to that.

So, we wind up with two very different results which may not be what you actually want. In this case, though, we only get an empty string where we may expect some default text. Worst case scenario, our app displays some blank text. Annoying, yeah, but not the end of the world.

But wait, it gets worse.

If you use logical or assignment with interfaces, things get really crazy. Here’s another test class file to test:

package 
{
	
import flash.display.Sprite;
import flash.events.Event;

public class Main extends Sprite 
{
	
	public function Main():void 
	{
		if (stage) init();
		else addEventListener(Event.ADDED_TO_STAGE, init);
	}
	
	private function init(event:Event = null):void 
	{
		removeEventListener(Event.ADDED_TO_STAGE, init);
		
		var f:Test = new Test("blah");
		testClass(f);
	}
	
	private function testClass(someArg:ITest=null):void
	{
		someArg ||= new Test("yadda");
		
		trace("someArg=(" + someArg + ")");
		// output:
		// ASC1: someArg=(Test [foobar=blah])
		// ASC2: someArg=(-2.1750519003895844e-311)
		
		var name:String = someArg.foobar;
		
		trace("Name=(" + name  + ")");
		// output:
		// ASC1: Name=(blah)
		// ASC2: Null Object Error is thrown at line above
	}
}
	
}

interface ITest
{
	function get foobar():String;
	function toString():String;
}

class Test implements ITest
{
	private var _foobar:String;

	public function Test(foobar:String)
	{
		_foobar = foobar;
	}

	public function get foobar():String { return _foobar; }

	public function toString():String
	{
		return "Test [foobar=" + this._foobar + "]";
	}
}

Let’s look at the byte code for ASC1 again.

18   getlocal1     	
19   coerce        	private::ITest //nameIndex = 10
21   dup           	
22   convert_b     	
23   iftrue        	L1

27   pop           	
28   findpropstrict	private::Test //nameIndex = 8
30   pushstring    	"yadda"  //stringIndex = 23
32   constructprop 	private::Test (1) //nameIndex = 8
35   coerce        	private::ITest //nameIndex = 10

L1: 
37   coerce        	private::ITest //nameIndex = 10
39   setlocal1     	
40   debugline     	28
42   findpropstrict	trace //nameIndex = 11
44   pushstring    	"someArg=("  //stringIndex = 25
46   getlocal1     	
47   add           	
48   pushstring    	")"  //stringIndex = 26	

We get our local ‘someArg’ variable, type it as an ITest object (‘coerce’), and convert that to a Boolean. Of course that boolean is true, so the ‘iftrue’ passes and we jump to the L1 block, type the variable to an ITest once again, set our local variable again, then do the trace.

But again, with the ASC2 compiler we see something very different:

14   getlocal1     	
15   iftrue        	L1

19   findpropstrict	private::Test //nameIndex = 12
21   debugline     	26
23   pushstring    	"yadda"  //stringIndex = 36
25   constructprop 	private::Test (1) //nameIndex = 12
28   setlocal1     	

L1: 
29   getlex        	trace //nameIndex = 17
31   getglobalscope	
32   debugline     	28
34   pushstring    	"someArg=("  //stringIndex = 38
36   getlocal1     	
37   add           	
38   pushstring    	")"  //stringIndex = 39

Again, with ASC2 we get our local variable but do no coercion or conversion, so again we wind up with an ‘untyped something’. But, again, an ‘untyped something’ is not nothing, so the ‘iftrue’ passes and we jump to the L1 block. This time though, we still don’t coerce the ‘untyped something’ into an object and instead jump right into the trace statement and we see that our ‘untyped something’ is actually represented as an infinitesimally small number in memory (see the output comments in the actionscript above). But as you can see in the comments above, when we try to access properties of this ‘untyped something’ we wind up with a null object error.

As you can imagine, compared to the minor annoyance of an empty string, a null object error is a real ball deal breaker.

I’ve filed a bug report on this behavior with Adobe which you can check out here. In the meantime, though, if you’re making the jump into AIR 3.8 (or AIR 3.7 with the ASC2 compiler), I highly recommend avoiding using OR assignment like the plague.

Date:
Category: