Assign to Boolean variable does not convert to true or false, breaking loose comparison with ==

[joshtynjala]

Allowing boolean to store anything except true and false breaks loose comparison with the == operator. That's much more serious than breaking strict comparison with the === operator!

var obj:Object = {};
var bool1:Boolean = obj;
var bool2:Boolean = true;

if(bool) //works
{
}

if(bool1 == bool2) //fails!
{
}

To coerce a value to boolean, we can use !! in JavaScript:

var obj = {};
var bool1 = !!obj; //true

This coerces to true with any type of object, and also coerces to false for things like null and undefined.

Initializing a boolean with another boolean (or an expression that resolves to boolean) doesn't require coercion.

Initializing a boolean with a number literal, null, or undefined should output true or false at compile-time.

Additionally, similar to how int and uint default to 0 when there's no initializer, Boolean variables should default to false unless initialized with another value.

[aharui]

IMO, the pattern of comparing two booleans is rare and it would not be my preference to see every boolean initialized to false just in case. Maybe we should consider warning on boolean compares and have an option as to whether to generate the initializer or not.

[Harbs]

@aharui I originally agreed with your position, but I've bumped into lots of weird bugs due to booleans not being initialized. That includes "undefined" ending up in some of my XML instead of "false".

Comparing two booleans is also not so uncommon. We're doing a lot of this in Spectrum: https://github.com/unhurdle/spectrum-royale/blob/master/Spectrum/src/com/unhurdle/spectrum/card/Card.as#L23 Manually typing !! should not be necessary in AS3.

One of the big selling points of AS3 is runtime type safety and I think that we need to keep that as much as possible.

booleans should definitely be initialized on the prototype rather than instances when possible to prevent memory inflation, but when initialized on the prototype, the expense of initializing them is practically free.

[carlosrovira]

I don't think comparing to booleans is strange, I think the opposite. I have lots of cases in my real code in different parts, and a great percentage of those are in bindings

[joshtynjala]

That includes "undefined" ending up in some of my XML instead of "false".

I was just thinking to myself that this is also potentially an issue for JSON. You don't want to stringify a truthy/falsy value instead of real true or false.

Comparing two booleans is also not so uncommon. We're doing a lot of this in Spectrum:

I was trying to think of some real world examples of comparing booleans, and inside a setter to avoid invalidation or dispatching an event is a perfect one! Pretty common, actually.

booleans should definitely be initialized on the prototype rather than instances

Agreed. They're passed by value, so initializing on the prototype shouldn't be an issue.

[aharui]

We could try to do a survey, but I'm pretty sure a significant number of booleans won't need to be initialized. So far, we've managed to get a lot of code running without initializing them. I think it is rare except in setters. I never said it was "strange".

AIUI, an uninitialized boolean takes zero bytes in the final output. The cost of initializing a boolean is at minimum, something like:

b.c=0; // b is 'a.prototype'`

PAYG philosophy encourages solving the actual problem instead of adding more code just in case, so one option may be that for

(someBoolean == someOtherBoolean)

We generate:

(||someBoolean == ||someOtherBoolean)

Or maybe even:

(someBoolean === someOtherBoolean)

Or offer choices on what to generate.

We could offer a warning as well so folks can consider how they want that code to work. Maybe they will just use the warning to manually initialize the backing variable in a setter.

Royale has a JSONReviver already, but may need a JSONReducer that doesn't just hand over the object tree to JSON, but adds other information like classAlias similar to how AMF works so that the right instances of ValueObject are created in reading/writing JSON. That JSONReducer could know that a field is boolean and handle it correctly.

FWIW, I've often thought about pre-processors and other template-like features for the compiler. It is always frustrating to have to type a new getter/setter pair. The compiler folks were very against preprocessors, but in some ways you could argue that the code generated by [Bindable] is effectively coming from a preprocessor. So a more complex solution could be to introduce [GetterSetter] metadata that takes an optional change detection method and have the compiler generate the right code including declaring and initializing the backing variable.

There is a half-way point where the compiler initializes backing variables used in compares by assuming the backing variable will have a name that matches the property name with a leading or trailing "_".

Also, FWIW, I've wondered if the compiler were to generate such code, whether the compiler should generate code that tries to reuse strings. I'm not sure how well ZIP compression works with string variants, but each setter with change detection usually has 3 string variants: the name of the property, the name of the backing variable and the name of the change event.

The point is that these ideas address the actual problem, which that uninitialized booleans have zero cost and work fine in most cases, but there are specific places where special handling is needed.

[joshtynjala]

It's also worth mentioning that this change will ensure that existing AS3 code continues to work properly when brought into Royale, which (as you frequently mention, Alex) is one of our goals. It would not be "just in case", but actually conforming to developer expectations of how the language has always worked.

Additionally, we already have int, uint, Number, and String being coerced on assignment because it's been found to be necessary for various reasons. It would actually be counter-intuitive if Boolean weren't given the same treatment.

This also gives ActionScript an advantage over TypeScript. This automatic coercion on assignment makes it harder in ActionScript for a variable to hold the wrong type, which makes things more predictable for developers. It's still possible to write code that will do it, of course, but it's hard enough that someone would have to do it with intent.

[Harbs]

b.c=0; // b is 'a.prototype'

I assume you mean b.c=!1

It's not just ==. You'd also need to do bool1 = !!bool2 for every single assignment because you never know if bool2 was initialized.

XML and JSON are two examples of cases where undefined produce undesired results. There's probably more. To me the "cost" is more the memory consumption than the cost in code, but cost in potential bugs outweighs both of those.

I don't understand what you mean with backing variables, but that doesn't sound free either.

[aharui]

AIUI, code brought into Royale from Flex will work for all booleans except those actually used in a boolean == boolean comparison.

AIUI, assignments would not have to be changed unless later that code is used in a boolean == boolean comparison.

We can and should provide XML and JSON "writers" that are knowledgable about types. I'm not convinced that booleans are the only issue that XML and JSON writers can be smarter about and that "dumb" writers will get wrong. I would venture to guess that any other scenarios we run into can be handled in the code that utilizes the boolean. That is also PAYG.

I believe we can solve the specific scenarios instead of generating lots of unnecessary initialization code "just-in-case".

A backing variable is the variable that actually stores the value in a getter/setter.

private var _foo:Boolean; // this is the backing variable
public function get foo():Boolean
{
   return _foo;
}

The backing variable can't be free if used in boolean==boolean compares. You have to pay for it when you need it. That's PAYG for you. But all other backing booleans can be free if they are never used in boolean==boolean compares. You do not always need change detection in a setter. Some ValueObjects can be "read-only" and are initialized by a network fetch and never change at runtime. Then the code can be as simple as the "foo" example above and there will never be a need to have initialized the backing variable.

IMO, this is a separate issue from the type-conversion changes for int/uint/number/string. This is about initialization of booleans, not assigning values of different types.

[greg-dove]

fwiw I am in the 'initialise' over 'optimise' camp in general too, to maintain as3 consistency. There are still enough other issues to deal with when porting apps, or using other code, that this type of optimization seems (to me) to be a trivial advantage. I think of the recent example with AMFBinaryData where code was working, but really should not have been (and was exposed by Josh' compiler updates for numeric type conversions) - because it was behaving like javascript and not actionscript - is a good example of why this type of thing should be on by default. Also, again fwiw, I think jx-compiler should not try to output things like !!0 or !0 etc here... it should be true or false. GCC does all that stuff in the release output optimisations... that's its job IMO.

[Harbs]

As a point of reference, I just audited all of the Royale framework code.

There's 1264 public, private and protected boolean variables declared across all the frameworks combined and this includes the code in all the examples. Actual apps will only use a small percentage of that.

My app which compiles to 189630 lines of code has a total of 525 booleans declared on prototypes (i.e. public, protected and private vars). Of those, 150 are already initialized. That means we're concerned about 375 initializations.

[Harbs]

Also, again fwiw, I think jx-compiler should not try to output things like !!0 or !0 etc here... it should be true or false. GCC does all that stuff in the release output optimisations... that's its job IMO.

Agreed.

[aharui]

I don't see PAYG as an "optimization" like other post-processing optimizations. It is just a way of making decisions to keep the framework as small and fast as possible.

If we agree that initializing a boolean will result in 7 or 8 bytes, then 375 initializations are over 2K. I can't tell you how often we said in Flex that an extra 1K or 2K wouldn't hurt. But it eventually all added up to 130K HelloWorld and 13,000 lines in UIComponent.

My very first comment on this issue recommended having an option as to whether to initialize booleans or not. I seem to recall a discussion on another topic where we were going to leave an option on by default but keep it off in the framework. I'm fine with doing that here.

[Harbs]

I seem to recall a discussion on another topic where we were going to leave an option on by default but keep it off in the framework. I'm fine with doing that here.

Seems like a reasonable compromise. We can make educated decisions on which booleans to explicitly initialize in the framework code.

[Harbs]

Oops. Hit the close button by mistake...

[joshtynjala]

I seem to recall a discussion on another topic where we were going to leave an option on by default but keep it off in the framework. I'm fine with doing that here.

This sounds good to me. 👍

[greg-dove]

'if we agree that initializing a boolean will result in 7 or 8 bytes, then 375 initializations are over 2K. I can't tell you how often we said in Flex that an extra 1K or 2K wouldn't hurt.' I think the difference there is that this scenario is whether or not the code is actionscript compliant in general, (with actionscript strong typing being a major advantage to claim over vanilla js).... vs. adding some sort of 'just-in-case' functionality to framework classes. But I also agree this makes sense to keep off in the framework and on by default otherwise... sounds great to me too.

[aharui]

Sounds like we have agreement on this particular topic.

I do want to post some thoughts about "actionscript compliance". IMO, it may never truly be practical to fully emulate the ActionScript runtime in the output and I think there are bigger fish to fry right now than to try to fully emulate everything the runtime can do. So to me, the argument for compliance or consistency is not a clear winner. The cost of the implementation in terms of code size and performance and committer time should be weighed against the likelihood of occurrences. Sometimes, we should encourage or even require our early adopters to change their code.

The particular case I'm thinking of is the generic property read or write. Right now, if you have code like:

function foo(bar:Object):void {
bar.baz = 2;

The compiler/transpiler will generate:

bar.baz = 2;

But that will not work in the browser if bar is XML or Proxy. It will work in the Flash runtime. I would not be in favor of replacing all property accesses with a call to some utility function that checks the type and makes the right call like the Flash runtime does. IOW, to have some thing like:

function Language.write(o:Object, prop:String, value:*):void
{
   if (o is Proxy)
     o.setProperty(prop, value);
   else if (o is XML) {
     if (prop.charAt(0) == "@")
        o.setAttribute(prop, value);
     else
       o.replace(prop, value);
   }
   else
     o[prop] = value;
} 

And then generate:

Language.write(bar, "bar", 2);

I wouldn't want our default output to look like that and run each property read and write through another function call. I don't think that would be the right first impression to give people. Folks migrating are currently required to rewrite the code and inject the tests they need or strongly-type "bar" if they always know what it will be. And that might need to be "good enough" for now.

[greg-dove]

For things like this it might also be useful to consider what has been done or is being done elsewhere. I think with React it is becoming more popular to use Typescript for type definitions now. But it also has a way to 'type' properties, with support for runtime type checking using 'proptypes'. You can get a bit of a sense of that here: https://reactjs.org/docs/typechecking-with-proptypes.html

It does the type of thing that you described above as runtime type checking with the expected overhead, but this is only present in the dev/debug build. It is completely absent from the release build.

I'm not saying we prioritize something like this, as I assume it could be time-consuming to get right, but something like this, if introduced in the future, could provide the same type of type safety as the player in debug builds, allowing for runtime checking, but have zero cost for the release build where it is absent. Maybe something like this could even diagnose issues with boolean coercion at runtime in the debug build and identify specific cases where initialization to false was required.

Anyway... for me the 'actionscript compliance' is more about predictability/reliability (as a language), and I consider this ahead of performance, and I don't think I would be alone in that. But I am also sure there is a spectrum of opinion/views. In the end I think if something seems to not work the way it should (based on established rules for the language) then it can be perceived to be unpredictable or unreliable.

I'll leave it there... I just wanted to mention the possibility of this type of runtime thing as debug-only. For now, as you said, I think we have a good decision on this topic.

On Fri, Feb 8, 2019 at 4:00 PM aharui notifications@github.com wrote:

Sounds like we have agreement on this particular topic.

I do want to post some thoughts about "actionscript compliance". IMO, it may never truly be practical to fully emulate the ActionScript runtime in the output and I think there are bigger fish to fry right now than to try to fully emulate everything the runtime can do. So to me, the argument for compliance or consistency is not a clear winner. The cost of the implementation in terms of code size and performance and committer time should be weighed against the likelihood of occurrences. Sometimes, we should encourage or even require our early adopters to change their code.

The particular case I'm thinking of is the generic property read or write. Right now, if you have code like:

function foo(bar:Object):void { bar.baz = 2;

The compiler/transpiler will generate:

bar.baz = 2;

But that will not work in the browser if bar is XML or Proxy. It will work in the Flash runtime. I would not be in favor of replacing all property accesses with a call to some utility function that checks the type and makes the right call like the Flash runtime does. IOW, to have some thing like:

function Language.write(o:Object, prop:String, value:*):void { if (o is Proxy) o.setProperty(prop, value); else if (o is XML) { if (prop.charAt(0) == "@") o.setAttribute(prop, value); else o.replace(prop, value); } else o[prop] = value; }

And then generate:

Language.write(bar, "bar", 2);

I wouldn't want our default output to look like that and run each property read and write through another function call. I don't think that would be the right first impression to give people. Folks migrating are currently required to rewrite the code and inject the tests they need or strongly-type "bar" if they always know what it will be. And that might need to be "good enough" for now.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/apache/royale-compiler/issues/78#issuecomment-461674685, or mute the thread https://github.com/notifications/unsubscribe-auth/AEW31huXaA_myUt8p3-ZgdOme3T_VeFfks5vLOhHgaJpZM4amRoa .

[aharui]

@greg-dove. Your correction appeared on the dev-list but not here, which is that handling different read/write accessor functions has to stick around in production code. It can't be debug-only.

Type-mismatch checking is a different problem and could be debug-only. There might be some clever ways we can make that happen by swapping in different Language.is/as methods. That said, type-mismatch and interface incompatibility checks are reasons why the original vision for FlexJS/Royale included using Flash/AIR to test your code paths, where the runtime will catch more of these things.

[joshtynjala]

I have added the boolean coercion. I'll turn on initialization by default (and update the framework to turn it back off) when I'm back from vacation.

[greg-dove]

@aharui Sorry about that, I guess I realized the difference after reading the email notification and replied directly in response to the email notification. I will be more careful in future.