rubyjs / therubyracer

Embed the V8 Javascript Interpreter into Ruby
1.67k stars 190 forks source link

add support for TryCatch along with StackTrace #379

Closed cowboyd closed 8 years ago

cowboyd commented 8 years ago

This adds support for running a v8::TryCatch block in Ruby by passing in a block. As part of the TryCatch api, it was also necessary to add the StackTrace StackFrame and Message apis as well.

Since constants cannot be defined in ruby with lower case names, constants such as V8::C::StackTrace::kOverview are implemented as methods with the ClassBuilder::defineConstMethod

georgyangelov commented 8 years ago

This is really nice! :) I would have just capitalized the first letter of the constants, but this works, too. What was the problem with the Wrapper template?

cowboyd commented 8 years ago

Yeah, I was conflicted about it, but in the end decided to try and keep it as close the the underlying V8 names as possible.

The problem with the Wrapper template is because v8::TryCatch has no copy constructor, and so if you are going to pass it to Wrapper, then you need to pass it by reference, which means that I needed to change the Wrapper constructor to:

Wrapper(T& content) : container(new Container(content)) {}

// and then in the container
struct Container {
  Container(T& content_) : content(content_) {}
  T& content;
}

Which worked for TryCatch, but then it started complaining about const PropertyCallbackInfo& info losing its const assignation if I try to pass it into a constructor that took a PropertyCallbackInfo& info parameter (which is what the Wrapper template generated). I guess since I had been passing by value, it was creating a copy of the PropertyCallbackInfo which I was free to modify, but if it was passed by reference, then it got all crazy about making sure it was never changed.

You know, I never did try just having two constructors for Wrapper and Container one that was pass by value, and the other pass by reference.

cowboyd commented 8 years ago

Hmm, adding a different constructor didn't work, but I did almost get it by putting the reference declaration in with the template parameters:

class TryCatch : public Wrapper<v8::TryCatch&> {
};

However, the problem comes with the dereference operator:

../../../../ext/v8/wrapper.h:64:13: error: 'operator->' declared as a pointer to a reference of type
      'v8::TryCatch &'
    inline T* operator ->() {

because the generatoed operator would look like V8::TryCatch&* operator ->(); which I guess doesn't make much sense.

I could probably get it work, but it would mean removing the dereference operator and adding a * operator It would be a bit uglier:

T operator*() {return container->content;}
TryCatch trycatch(self);
(*trycatch).HasCaught();

Makes me wonder if it's worth it :/

cowboyd commented 8 years ago

I'm going to go ahead and pull this in. If you want to see if you can get it to work with Wrapper, I'd be more than happy to see what you come up with, but it's got me stumped.