Disabling assertion checking at runtime.

[CeylonMigrationBot]

[@gavinking] So we recently did our first totally nasty hack to the language module (Element|Finished elementAt(Integer index)) for the purpose of avoiding the cost of a runtime type assertion check. That's a bad sign, and an indication that something's wrong. I know I've argued that assertions should be "always on", and I still think that's true for most typical code. But maybe it's not true for very well-tested, performance-sensitive library code like in the language module.

Therefore, I'm think perhaps we should add some way to disable runtime assertion checking. Either:

1. via a compiler switch when the code is compiled, or, perhaps,
2. by annotating the assertion unchecked.

Or perhaps even via a combination of 1 and 2.

WDYT?

[Migrated from ceylon/ceylon-compiler#1677]

[CeylonMigrationBot]

[@lucaswerkmeister] “very well-tested” code requires that the assertion was enabled during development and testing, so I think a compile-time mechanism makes more sense.

[CeylonMigrationBot]

[@gavinking] @lucaswerkmeister yes, or, as I said, a combination of the two switches, where both have to be enabled.

[CeylonMigrationBot]

[@lucaswerkmeister] Hm, you’re right, there are of course assertions that you always want (regular “is the argument valid?” checks). So I guess 1+2 makes the most sense.

[CeylonMigrationBot]

[@tombentley] If you compile with unchecked assertions disabled, but the assertion is not valid, then we open the door to some weird behaviour at runtime. Rather than getting a nice AssertionError with plenty of context about what failed we'll get things like NPEs and CCEs thrown by the JVM at some later evaluation. For example

unchecked assert(exists item = list[index]);

might transform to:

Element item = list.get(index);
// no null check

and then a later expression on item will throw an NPE.

We can say that that's the users fault for using an unchecked assert(), but this is not a local problem: I might depend on a module with an unchecked assert() and then find my code blows up because of someone elses bogus assert(). That really sucks for modularity and coding in a team environment.

We can mitigate that somewhat by deferring the decision about checking unchecked assertions until runtime. Then at least when I test my module, I can enable unchecked assertions in everyone elses modules and more easily find the cause of the bogus assertion.

[CeylonMigrationBot]

[@gavinking] @tombentley Well, I mean the very worst thing that can possibly happen is that your Ceylon program does what a Java program would do in exactly the same case :-)

[CeylonMigrationBot]

[@tombentley] Yes, but that's not actually very good, is it? We go to all this effort to try to get rid of NPEs and their detestable property of non-locality, only to subvert all that work have all that work subverted by someone unwisely deciding to compile their module with unchecked assertions disabled.

[CeylonMigrationBot]

[@lucaswerkmeister] Perhaps you’d need to annotate the module unchecked as well to allow unchecked asserts, and you’d also need to annotate the module import unchecked?

Or perhaps this is something that can be solved at the assembly level: The assembler (person) would decide to use my.module-1.2.3-UNCHECKED as my.module-1.2.3. (This, of course, relies on people publishing both checked and unchecked versions of their modules.)

[CeylonMigrationBot]

[@tombentley] @gavinking is this still something we need to do for 1.1?

[CeylonMigrationBot]

[@gavinking] Perhaps not. I still want to do a perf test of the diff b/w iterating a arrays/sequences containing nulls and arrays/sequences containing non-nulls.

[CeylonMigrationBot]

[@gavinking] So my initial tests show that having nulls in the collections adds a small (~15%) overhead compared to having no nulls in an ArrayList, and about a 30% overhead in a plain Array. (Can't explain the behavior of Array since it doesn't do the assertion, AFAIK.)

Whatever, looks like the assert (is Element null) is not expensive, thanks to @tombentley's optimization. Good.

So this doesn't need to be in 1.1.