[cfe-dev] Fwd: [libc++] Should we guard against macro definitions

[James Y Knight via cfe-dev]

Certainly libstdc++'s cmath header has a ton of "#undef abs", "#undef div",
etc. lines at the beginning of the file.

It seems likely that would be a sensible thing to do in libc++ too.

On Wed, Sep 30, 2015 at 4:56 PM, Ben Craig via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> Short story:
>
> Instead of putting lots of parenthesis everywhere, I think the better
> approach is to put lots of #ifdef + #undef blocks everywhere.  More
> rationale follows.
>
>
>
>
>
> That's not good enough, because we have to "hoist" the function into
> namespace std.
>
>
>
> Yes and no.
>
>
>
> First the good(?) part.  This C implementation starts with a real function
> definition for ‘cos’, then follows it up with the macro version.  That
> means that “using ::cos;” does get the “double cos(double)” signature into
> namespace std, because it doesn’t invoke the macro (no parenthesis).
>
>
>
> The bad part is that the cos macro is still hanging around, and if someone
> types something reasonable like “float x = cos(1.0f);” they will invoke the
> double version.  If they type “float x = std::cos(1.0f);” they will get a
> gross compiler error.
>
>
>
> This suggests to me that the better answer is to throw around a lot of
> blocks of the form #ifdef cos, #undef cos.
>
>
>
> libc++'s <cmath> header does this for some of the calls there (look at
> 'fpclassify', for example).
>
>
>
> Doing the same for cos (and others) shouldn't be too hard.
>
>
>
> I think C99 fpclassify is specified to be a macro, and C++ fpclassify is
> specified to be a set of functions.  Given that, I think the #undef
> technique used there is reasonable.  I don’t think we can do the same for
> the math functions though, because if we #undef the old cos, then introduce
> a new cos overload that takes a double as the only argument, then we will
> introduce ambiguity.  If we just do the #undef though, then I think we’ll
> be fine, as long as the cos macro isn’t the only way of getting to the C
> libraries cos implementation.
>
>
>
> Can you provide a list of the functions that your library defines as
> macros?
>
>
>
> I’m not sure if I can, but either way, it’s a rather spotty list.  For
> example, sinhf has a macro version, but sinf doesn’t.  I think it may make
> more sense to look at everything in the C version of tgmath, and guard all
> of those.  I’m absolutely willing to be the one to make the patch, I was
> just trying to gauge interest before going through the motions.
>
>
>
>
>
> Employee of Qualcomm Innovation Center, Inc.
>
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
>
>
>
> *From:* cfe-dev [mailto:cfe-dev-bounces at lists.llvm.org] *On Behalf Of *Marshall
> Clow via cfe-dev
> *Sent:* Wednesday, September 30, 2015 2:01 PM
> *To:* cfe-dev at lists.llvm.org
> *Subject:* [cfe-dev] Fwd: [libc++] Should we guard against macro
> definitions
>
>
>
> forgot to cc the list on my reply.
>
>
>
> -- Marshall
>
>
>
> ---------- Forwarded message ----------
> From: *Marshall Clow* <mclow.lists at gmail.com>
> Date: Wed, Sep 30, 2015 at 11:58 AM
> Subject: Re: [cfe-dev] [libc++] Should we guard against macro definitions
> To: Ben Craig <ben.craig at codeaurora.org>
>
>
>
>
>
> On Tue, Sep 22, 2015 at 9:44 AM, Ben Craig via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
> I have a C library, and I want to build libc++ and libc++abi against it.
> This C library’s implementation of many of the math.h functions involves
> code similar to…
>
> #define cos(Val) RealCosImpl(Val)
>
>
>
> This interacts poorly with libcxx’s cmath header, as the float and long
> double overload declarations hit this #define.
>
>
>
> If I make a patch that guards the bulk of cmath from this kind of macro,
> do people feel that this would be useful, or would it get rejected on the
> grounds that it is unnecessary noise to support an odd C library?  This fix
> would generally take the form of putting the function name in parenthesis
> to suppress function macro expansion.  So code that looks like this…
>
> float cos(float val)
>
> … would turn into this…
>
> float (cos)(float val)
>
>
>
> That's not good enough, because we have to "hoist" the function into
> namespace std.
>
>
>
>
>
> One could argue that this would be a reasonable thing to do across libcxx,
> but I think it is more important to do so for cmath, as many of those
> functions must have macro version in the C tgmath.h header (but not the C++
> one).  It probably wouldn’t hurt to throw some parenthesis at std::min and
> std::max in <algorithm> as well, considering the unfortunate history with
> those and the <windows.h> min and max macros.
>
>
>
> libc++'s <cmath> header does this for some of the calls there (look at
> 'fpclassify', for example).
>
>
>
> Doing the same for cos (and others) shouldn't be too hard.
>
> Can you provide a list of the functions that your library defines as
> macros?
>
>
>
> -- Marshall
>
>
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150930/babae52b/attachment.html>