[libcxx-commits] [PATCH] D142806: [libc++] Implement P0226R1 (Mathematical Special Functions)

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D142806#4112740 <https://reviews.llvm.org/D142806#4112740>, @lattner wrote:

> I'm sorry, but accepting this patch will be very difficult for the project.  I have several technical concerns with this patch:
>
> 1. This is a very large block of code which does not follow LLVM conventions and has a lot of boost support functionality that seems redundant with stuff elsewhere in LLVM.
> 2. This seems to overlap a lot with the llvm libc work, particularly for the simple numeric operations.
> 3. This appears to add code with new dependencies, e.g. gmpfrxx? and has internal abstractions that aren't necessary for libcxx

I have to respectfully disagree with those technical concerns. Setting aside licensing issues, it shouldn't be a problem to use a third-party library to implement complex functionality -- in fact we already have precedent for that: we use Ryu and MSVC's code to implement `std::from_chars` and we use GoogleBenchmark for our benchmarks. Some things in the Standard Library are so complex that we literally have to rely on a third-party implementation if we are to ever ship it -- we just don't have the resources to do it ourselves. Furthermore, I don't think anything else inside LLVM implements those mathematical special functions, and the C Standard Library doesn't have those, so I don't think it will make sense for LLVM's libc to provide them in a relatively near future.

The way we've done that without compromising libc++'s integrity is by making sure that the third-party code stays an implementation detail of the built library (shared or static), so that users don't end up including that third-party code when they include libc++. And by hiding it behind an ABI boundary that way, we are free to swap the underlying implementation for something else in the future at any time. Modulo a few details, this patch is basically the way I had always envisioned integrating the math special functions into libc++.

> Furthermore, the use of the boost license is a big issue.  While I am not a lawyer, it appears that it could be "compatible" with the LLVM license, but this is not enough.

I think this is the main issue at play. I do think it is compatible, however the plan has always been to ask the Boost folks to relicense it under the LLVM license, and I think we should definitely do that.

So assuming this were under the LLVM license, I'd be quite happy with taking this patch (after review and a couple changes). @lattner In light of what I said above, are you OK with this plan given that we get everything licensed under the LLVM license?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142806/new/

https://reviews.llvm.org/D142806