Make Exception a subclass of NSException (wip) #25

Open
wants to merge 8 commits into
from

Projects

None yet

3 participants

@alloy
MacRuby member

First let me say that the code, as it is, is not meant to be pulled yet, but I thought it might be best to keep the discussion in this one place.

This is a first pass at making the Ruby Exception class a subclass of the Objective-C class NSException.

Notes/questions:

  • What to do in 32-bit env? I understood that in this case we don't use the C++ exception mechanism, however I wonder if that matters for having Exception as a NSException subclass. I.e. apart from raising an exception etc it should just be a normal class right? So it shouldn't hurt to have Exception be a subclass of NSException on 32-bit for consistency as well?

  • The only failing spec is Exception#exception returns an exception of the same class as self with the message, which (iirc) is because the instance is being cloned. This probably means that initialize_copy needs work?

  • Should we use the NSException#reason property to store the Exception#message? Afaik it should be a string in Ruby as well, but I'm not 100% sure.

  • Doing rescue Exception means that NSException will not be rescued, because it's the superclass. However, I wonder if that's a real problem for existing Ruby code. If, however, that's not an option, then the only solution I see is to make Exception an alias for NSException with extra behavior.

@alloy
MacRuby member

To be clear, the 32-bit code I removed will be re-added again.

@lrz
MacRuby member

To answer your questions:

1) The fact that the C++ exception mechanism isn't used in 32-bit doesn't matter here, as we will use the ObjC runtime, which falls back to SjLj when ran in 32-bit.

2) Maybe, but that seems minor. I prefer to see a version of the patch integrated into master before tackling these edge cases.

3) I would rather not. But Exception#message can poke at NSException#reason in case there is no message (if it's a pure ObjC exception). Again, that seems minor here, let's integrate a first version of patch before.

4) Exception should be an alias to NSException, and we should introduce a class like RubyException for Ruby-raised exceptions.

@lrz
MacRuby member

As for the patch itself, it's not usable right now because you removed 32-bit code. Please add it back and make it work, then we can start iterating on the patch :)

@alloy
MacRuby member
  • Should the NSException category move to error.c, or is this the right way?
  • Should we also override -[NSException callStackSymbols] to return the Ruby backtrace if it exists?
@alloy
MacRuby member

So would rb_eRubyException be a subclass of Exception/NSException but the superclass of all in Ruby defined exception classes? E.g. StandardError.

And how does that work when people subclass Exception? Would we under the hood actually subclass rb_eRubyException instead?

@alloy
MacRuby member

And lastly:

Regarding: https://github.com/alloy/MacRuby/blob/nsexception/compiler.cpp#L6876
Should we just use the body of rb_rb2oc_exc_handler here directly?
Because we no longer need to convert the exception, just throw it.

Regarding: https://github.com/alloy/MacRuby/blob/nsexception/vm.cpp#L3563
How does this work? It seems that no information is set on the allocated exception.

@ferrous26
MacRuby member

I think we missed the one year anniversary of this pull request. :P

@alloy
MacRuby member

lol

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment