Macro brainstorming for Maybe<>/MaybeLocal<>

[domenic]

Dealing with Maybe<>s and MaybeLocal<>s in V8 4.3 are a pain, as I found out in #1773 and @trevnorris found out in #1825. This could maybe be made better with macros.

Here is one proposal:

// Original code
MaybeLocal<Object> maybe_foo = get_foo_from_v8();
Local<Object> foo;
if (!maybe_foo.ToLocal(&foo)) {
  return;
}

// Desired code
UN_MAYBE(foo, Object, get_foo_from_v8());

This would introduce a local variable foo of type Local<Object> (and another one maybe_foo of type MaybeLocal<Object>) in the exact same way as the above code. Either the function will have returned, or foo will contain a non-empty Local<Object>. I am 95% sure this is easy to write a macro for.

Ideas/issues:

• It would be nice if it could figure out the Object part for us. Can it, perhaps using auto or decltype or something?
• It'd be nice to write this as Local<Object> foo = UN_MAYBE(get_foo_from_v8()). Does anyone have good enough macro skills for that?
• I guess if we want to use this inside functions that don't return void, we'd need a return type too.
• V8 does stuff that's a bit more complicated (focus on PREPARE_FOR_EXECUTION_GENERIC and RETURN_ON_FAILED_EXECUTION + EXCEPTION_BAILOUT_CHECK_SCOPED, as well as various instances that set has_pending_exception throughout the code). Is there a good reason for that? Should we do something more like that?

[bnoordhuis]

It would be nice if it could figure out the Object part for us. Can it, perhaps using auto or decltype or something?

You can with gcc and clang but I don't know about MSVC 2013, it's notoriously bad at inferring types. Easy to test though.

It'd be nice to write this as Local<Object> foo = UN_MAYBE(get_foo_from_v8()). Does anyone have good enough macro skills for that?

You can use ({ ... }) macro syntax in gcc and clang but I don't think MSVC supports anything like that. (Also, hiding flow control like that is extremely nasty.)

I'd take the more explicit approach, like V8 does.

[benjamingr]

Typically, Maybes are used with binds (like a then) in functional languages to avoid messy code. This would look like:

get_foo_from_v8().then([](auto foo) {
    // only gets executed if the maybe contains the value, doesn't get executed otherwise
});

However, that doesn't seem to fit very well with the code structure and probably has a performance penalty. The problem is that putting returns in macros is considered really bad practice in C and C++.

[kkoopa]

Macros that declare variables are usually clumsy, as they cannot, say, be used as rvalues.

Real functional languages usually also offer pattern matching and lazy evaluation, which makes it easier to unwrap and to make functions accept maybe types as well as unwrapped ones. Here, writing a bunch of overloads for every possible Maybe-argument in every function is not that pleasant.

While somewhat clumsy, the way things are is better than the possible alternatives. This stuff cannot really be papered over in a "good" way.

[benjamingr]

Well, pointing out that most compilers (GCC, clang and MSVC) support and optimize (inline) lambdas, so it's possible to do:

template< class T, class S > 
MaybeLocal<S> bind_maybe(MaybeLocal<T>& maybe, std::function<MaybeLocal<S>(T)> fn) {
    Local<Object> x;
    if (!maybe.ToLocal(&x)) {
        return MaybeLocal<T>();
    }
    // if we want sugar, we can also accept a function that returns an S and not a MaybeLocal<S>
    // which would be similar to how promises allow returning both a promise or a value which 
    // gets wrapped.
    return fn(x); // pass a reference instead of a value?
}

Which would let you do:

bind_maybe(get_foo_from_v8(), [](Local<Object> x){ // or auto x?
    // do work on x, needs another overload for `void` 
    return someWorkOn(x);
});

[Fishrock123]

Is this actually reasonable? If not, can we close?

[trevnorris]

I think we first need to decide how we want to deal with these cases. Currently it's common to simply abort if something fails. e.g. if malloc fails. But now there's a way to propagate these types of issues, should we? It's also possible to remove all TryCatch.

Basically we need more planning, but using macros to deal with the new API would be helpful.

[domenic]

I think @benjamingr's idea is pretty neat, now that I know compilers inline those correctly. I would be interested in how much of an improvement it is in the real world. @benjamingr, if you have time, do you mind taking a Maybe-heavy function or two (node_contextify.cc has a few simple ones) and doing a before/after?

[benjamingr]

@domenic sure but none of the functions in node_contextify (at least on master) have any MaybeLocals yet whose syntax I can attempt to change.

Let's look at compiling, running a script and returning the result for example.

Looking at Script for example - let's say we have a Handle<String> already and a MaybeLocal<Context> we got and we need to run the code which means calling Compile and then Run. With the original version this would be:

auto maybe_context = magically_get_context();
auto source = magically_get_source_to_run();
Local<Context> context;
if (!maybe_context.ToLocal(&context)) {
  return MaybeLocal<Value>(); 
}
auto maybe_script = Script::Compile(context, source);
Local<Script> script;
if (!maybe_script.ToLocal(&script)) {
  return MaybeLocal<Value>(); 
}
return script->Run(); 

Which bind_maybe this is done as:

auto source = magically_get_source_to_run();
auto script = bind_maybe(magically_get_context(), [](Local<Context> ctx){ 
    return Script::Compile(ctx, source);
});
return bind_maybe(script, [](Local<Script> script) { return script->Run(); });

The transition of a MaybeLocal to a MaybeLocal is in fact the transition all functional languages do.

[domenic]

@benjamingr sorry, I meant on next...

Anyway, that does look pretty nice. I'd be interested in a few more examples, and other contributors' opinions, but I'd be pretty happy.

[benjamingr]

Hmm, that only contains a single use of MaybeLocal and in that single instance the above function doesn't help much (no error handling, only non-empty stuff is mapped), I can write a function that also does error handling if you'd like to see that version.

[domenic]

https://github.com/nodejs/io.js/blob/next/src/node_contextify.cc#L350-L375 is kind of what I was thinking... https://github.com/nodejs/io.js/blob/next/src/node_contextify.cc#L392-L418 has stuff for Maybe<>, not just MaybeLocal... but yeah, hmm, this might be harder until we have some more examples...

[bnoordhuis]

Closing due to inactivity.