Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code in embedding documentation does not compile. #657

Open
dgatwood opened this issue Jul 6, 2022 · 0 comments
Open

Code in embedding documentation does not compile. #657

dgatwood opened this issue Jul 6, 2022 · 0 comments

Comments

@dgatwood
Copy link

dgatwood commented Jul 6, 2022

The doc in question is here:

https://v8.dev/docs/embed

This line:

v8::Persistent<v8::Context> context =
    v8::Context::v8::New(isolate, nullptr, global_templ);

is quite obviously wrong, because the extra "v8::" doesn't belong, though the fix for part of that is obvious.

But this line and the line above:

v8::Persistent<v8::Context> context =
    v8::Context::New(isolate, nullptr, global);

are also more subtly wrong and won't compile, failing with this error:

error: no viable conversion from 'Local<v8::Context>' to 'v8::Persistent<v8::Context>'

and there are a number of other compile errors, too.

The fixes that I've seen so far are (to the best of my memory):

  • Mark it as Local instead of Persistent.
  • Call context->enter() so that the context doesn't go away when it goes out of scope.
  • In at least one spot, remove an extraneous "v8::" (v8::Context::v8::New).
  • Add ->ToLocalChecked() pretty much everywhere a string gets constructed.

Additionally, it would be enormously useful if the Hello World sample were just slightly more complete so that it is more helpful for people trying to embed V8 — specifically:

  • Move the code that creates the context into a separate function so that we can see a real-world example where the context and isolate go out of lexical scope.
  • Call isolate->enter() so that the isolate doesn't go away.
  • Call context->enter() so that the context doesn't go away.

These small changes would have saved me many hours of swearing trying to figure out why a seemingly simple bit of code was segfaulting, and trying in vain to find ways to convert a Local handle to a Persistent handle and back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant