Improve usability of assert/abort

[joshuawarner32]

The current requirement to have a System instance in order to call abort on makes it very difficult to write "clean", modular utilities. Anything that could possibly want to assert needs to have either an Aborter (of which System is the only current implementation) or a System parameter. If we want to add an assert somewhere that we don't have one of those, we need to add such a parameter.

Let's make the choice of abort functionality a link-time option rather than a runtime option.

We make sure to include our abort function (which I've called "crash" in #171) in a separate .cpp file from most everything else. Anyone wanting to swap out the implementation can do so at compile time. We can even build a library that has that symbol undefined and expects the executable itself to define it.

The only thing we lose in that case is the context of which VM instance this abort is associated with - and that can be solved pretty simply with thread locals.

[dicej]

I disagree that the VM instance an abort is associated with can be determined simply using thread locals. There's no reason why, for example, a JNI method couldn't create a new VM in the same process and thread and use it to complete its work. Trying to override and restore thread locals at every VM entry and exit point seems like at least as much work (and much harder to debug) as just passing the aborter to any code that needs it. I think we should avoid "hidden" arguments passed via the execution environment (which is what a thread local is). It hides data flow dependencies and impedes readability.

[joshuawarner32]

Trying to override and restore thread locals at every VM entry and exit point seems like at least as much work (and much harder to debug) as just passing the aborter to any code that needs it.

I disagree; code to enter/exit a VM context is very localized. Code that operates within a VM context is everywhere. So instead of having the complexity in one file, we get to feel it everywhere. I'd be happy to do the legwork in demonstrating that we can catch all the enter/exit cases without adding undue complexity or maintenance burden.

To be clear, I think passing a Thread* context around works fine when you're doing stuff in a very monolithic style, where you probably already have some sort of a context object everywhere you could possibly need it.

This isn't the case in more separate, modular, utility classes. Slice<T> comes to mind - I'd really like to check bounds in there with an assert - but I really can't justify an extra field for an Aborter in such a small object (only 2 machine words). And if we start passing around lots of little objects like that (which I want to), the parameter lists start to get really cluttered - either at the machine level, if they're passed implicitly in the objects, or explicitly in all the little tiny methods.

[joshuawarner32]

@dicej, what would you say to replacing just our assert (not expect or abort) with the standard assert (from C's <assert.h>)? This way we can keep the same principle of being able to customize abort behavior for release builds, but there will be substantially less friction in using asserts.

[dicej]

I'd have two concerns about that:

• On Windows, I've had a lot more luck getting a backtrace in GDB from a null pointer dereference than from an abort (which is what the standard assert uses). This could be addressed by defining our own version of assert which takes no Aborter but calls crash on failure.
• There would be extra overhead to converting an assert to an expect, and we'll be less likely to use expect when it's the right choice but there's no Aborter handy.

I'd really rather see us pass an Aborter to any code that might want it, or else have that code return an error (or throw an exception) to the calling code which has the option of aborting or recovering somehow.

[joshuawarner32]

The first point is easy. Funny how I just introduced a function like that :).

When I'm debugging, I specifically don't want anything catching an assert except for my debugger - as soon as it's left the relevant frame, we've lost valuable context (yes, you could set a breakpoint at some strategic location - but that's a pain). Perhaps that's a good differentiator between expect and abort; we could declare that expect always throws a VMError (which has some small chance of popping up in production), and assert is for problems we shouldn't ever see outside of development. We should use expect for checking any and all low-frequency interfaces between big components, and assert elsewhere. These big component interfaces should always have the proper context for an expect anyway, so assert->expect conversion shouldn't be a problem.