[PATCH] D26671: Replace APFloatBase static fltSemantics data members with getter functions

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 13 09:02:11 PST 2016


Fair enough, I think my objection was mostly on the grounds that I'm not
familiar with the extractSymbols script, but if this patch doesn't do
anything that isn't already pervasive elsewhere, then I can't really object.

On Tue, Dec 13, 2016 at 9:00 AM Mehdi AMINI via Phabricator <
reviews at reviews.llvm.org> wrote:

> mehdi_amini added a comment.
>
> I don't have anything against this patch as is, but I don't know about the
> MSVC issue so I won't be able to help here.
>
> In https://reviews.llvm.org/D26671#613437, @zturner wrote:
>
> > I see.  My mild preference would be to remove as many C++isms as
> possible from exported interfaces.  So, in descending order of preference,
> this would be:
> >
> > 1. Make it return a `void*` instead of a `fltSemantics&`
> > 2. Make these global free functions marked extern "C"
>
>
> If you want a C-like API, you should extend the LLVM C API or write
> wrappers around the C++ API, I don't believe there is a wide agreement to
> "remove C++isms" from LLVM C++ APIs themselves (it is possible I
> misunderstood what you're suggesting here).
>
>
>
> ================
> Comment at: llvm/include/llvm/ADT/APFloat.h:143
> +  static const fltSemantics &PPCDoubleDouble();
> +  static const fltSemantics &x87DoubleExtended();
>
> ----------------
> Functions name in LLVM usually starts with a verb (getters start with
> `get`).
>
>
> https://reviews.llvm.org/D26671
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161213/bbd07b2b/attachment.html>


More information about the llvm-commits mailing list