andyarvanitis / purescript-native-cpp-ffi

C++ foreign export implementations for the standard library
MIT License
21 stars 8 forks source link

Handling unsafeCompareImpl? #1

Closed lettier closed 4 years ago

lettier commented 5 years ago

Hello @andyarvanitis,

I've been filling out some needed FFI for my recent purescript-native project.

I came across unsafeCompareImpl. Unfortunately, it is used in some key areas so I couldn't avoid it.

For now I've been using

exports["unsafeCompareImpl"] = [](const boxed& lt) -> boxed {
  return [=](const boxed& eq) -> boxed {
    return [=](const boxed& gt) -> boxed {
      return [=](const boxed& x_) -> boxed {
        const auto x = unbox<double>(x_);
        return [=](const boxed& y_) -> boxed {
          const auto y = unbox<double>(y_);
          return x < y ? lt : x == y ? eq : gt;
        };
      };
    };
  };
};

thinking double is the safest, unsafe type.

Is there a way to infer the type from boxed? pure-c uses a tag convention. Maybe boxed could have a tag member variable holding the name of the last known type of what was boxed?

The other ord implementations work well, it's just this untyped one.

Thanks!

:+1:

andyarvanitis commented 5 years ago

Yeah, please see this prelude pull request: https://github.com/purescript/purescript-prelude/pull/183

Maybe you can use my version of the prelude – which only differs from the official one in this one change – until they merge it: https://github.com/andyarvanitis/purescript-prelude

My current FFI files assume this change, so no other files are needed to get it to work.

This PR merge is one of the things I was waiting for before "announcing" again, actually.

Also, my previous backend had tags, but I elected to remove them during the redesign.

lettier commented 5 years ago

Thank you @andyarvanitis.

I thought I might have to maintain my own fork of the prelude but yours will work out nicely.

I asked about the PR in the slack channel but no one must have seen it. Hopefully the PR is merged soon so you can announce the updates to purescript-native. I was previously using pure-11, back when it was the same, but have recently switched.

:+1:

andyarvanitis commented 5 years ago

The official purescript-prelude merged my PR for this, and their release v4.1.1 has it. I'm going to leave this issue open for now for people's info, at least until v4.1.1 becomes the default.