Skip to content
This repository was archived by the owner on Dec 4, 2023. It is now read-only.

Conversation

@cowboyd
Copy link
Collaborator

@cowboyd cowboyd commented Aug 12, 2015

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
Copy link
Collaborator

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
Copy link
Collaborator Author

cowboyd commented Aug 12, 2015

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
Copy link
Collaborator Author

cowboyd commented Aug 13, 2015

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
Copy link
Collaborator Author

cowboyd commented Aug 13, 2015

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.

cowboyd added a commit that referenced this pull request Aug 13, 2015
add support for TryCatch along with StackTrace
@cowboyd cowboyd merged commit 5a1632a into upgrade-to-v8-4.5 Aug 13, 2015
@cowboyd cowboyd deleted the 4.5/try-catch-throw branch August 14, 2015 11:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants