pmed / v8pp

Bind C++ functions and classes into V8 JavaScript engine
http://pmed.github.io/v8pp/
Other
887 stars 119 forks source link

Support for std::variant / union types #149

Closed wflohry closed 2 years ago

wflohry commented 3 years ago

Hi,

I've implemented basic support for std::variant / union types into the C++17 branch. Within the conversion function, wherever possible, I've tried to limit the type construction within the convert<std::variant<Ts...>> class and instead defer to each convert<T>. For instance, the containsObjectImpl function only checks whether the object is in the registry, and if found calls the appropriate convert<T>::from_v8 function rather than converting directly. This helps ensure that any special functions with from_v8 (e.g. cloning, etc.) still work.

The arithmetic types present some complexity since C++ offers much more fine-grained arithmetic types than JavaScript, so the conversion seeks to find the most appropriate based on:

  1. Whether the value is integral, floating point, or boolean;
  2. Whether the value fits in the appropriate integral type (e.g. avoids inserting 256 into a uint8_t); and
  3. The order from left to right. (This does mean that std::variant<int32_t, int64_t> and std::variant<int64_t, int32_t> will behave differently for values in within their intersection, like the value 5.)

There are a few other caveats. Since JavaScript offers heterogeneous containers, it would be difficult to verify the most appropriate container for std::variant<std::vector<float>, std::vector<uint8_t>>, or really any std::variant<std::vector<T>, std::array<T, N>, std::tuple<Ts...>, ...>, etc. since checking every element of a large array could be costly. However, if that seems necessary it could be implemented.

As always, change requests / edits welcome, and thanks for creating the library :thumbsup:

pmed commented 3 years ago

Hi,

that's awesome idea and huge amount of work, thank you so much! I'm going to review and merge this PR after #148

pmed commented 2 years ago

Superseded by #158