[cfe-dev] LibC++ v3.8 - Problems with ISO C wrapper headers

[Eric Fiselier via cfe-dev]

Hi Martin,

If you disagree you'll need to cite (or change) the standard. IMO the
standard is explicitly clear on this topic.

15.2 [library.c] says:
> The C ++ standard library also makes available the facilities of the C
standard library, suitably adjusted to
> ensure static type safety.

D.3 [depr.c.headers] says:
>1. every C header, each of which has a name of the form name.h, behaves as
if each name placed in the standard
> library namespace by the corresponding cname header is placed within the
global namespace scope. It is
> unspecified whether these names are first declared or defined within
namespace scope (3.3.6) of the namespace
> std and are then injected into the global namespace scope by explicit
using-declarations (7.3.3).

/Eric


On Sat, Jan 23, 2016 at 12:50 PM, Martin J. O'Riordan via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> I disagree.  I'm not new to C++ either, I built my first C++ compiler in
> 1986 and have been involved in C++ standardisation for a very long time,
> and the recent changes are wrong.
>
> Including "math.h" should NOT introduce overloaded versions of 'fabs' or
> other functions at global scope.  It may only introduce the overloaded
> functions within "namespace std".  The recent changes overload these
> functions at global scope.  That is a bug.
>
> My tests are running now, and when they are complete I will propose the
> patch, but I can assure you that overloading global symbols from the C
> headers is not compliant with C++.
>
>         MartinO
>
> -----Original Message-----
> From: metafoo at gmail.com [mailto:metafoo at gmail.com] On Behalf Of Richard
> Smith
> Sent: 23 January 2016 16:58
> To: Martin.ORiordan at movidius.com
> Cc: Clang Dev <cfe-dev at lists.llvm.org>
> Subject: Re: [cfe-dev] LibC++ v3.8 - Problems with ISO C wrapper headers
>
> Hi Martin,
>
> On 1/23/16, Martin J. O'Riordan via cfe-dev <cfe-dev at lists.llvm.org>
> wrote:
> > While getting our implementation of the compiler ready for v3.8 we
> > came across a significant problem in the new implementation of
> > LibC++'s handling of the ISO C Standard headers that needs to be fixed
> > before the v3.8 release.
>
> You're mistaken about a lot of the things you say below, but I agree that
> your example code ought to work, assuming that libc++ intends to support
> __fp16 (but I don't think it currently does). See the bottom of my mail for
> a suggestion of how to move forward with this.
>
> > We discovered the problem when running some tests that were derived
> > from real-world code.  The initial example involved a C++ program
> > including '<math.h>' and using 'fabs' but the issue applies equally to
> > the other headers, for example:
> >
> > #include <math.h>
> >
> > ...
> >
> > __fp16 y;  // We use FP16 a lot
> >
> > fabs(y);   // really -> fabs((double)y);
> >
> > In C there is only one 'fabs' with the signature:
> >
> > double fabs(double)
>
> That's largely irrelevant. In C++, <math.h> is a C++ standard library
> header, whose contents are specified by C++'s [c.math] and
> [depr.c.headers]. These say that <math.h> provides the same set of
> overloads in the global namespace that <cmath> provides in namespace std.
> libc++ has recently changed to implement this rule (that has
> *always* been part of the C++ standard). Thus your suggested approach to
> fixing this is not appropriate.
>
> > With the new implementation of the headers for LibC++, there is now a
> > file named 'c++/math.h' that includes the C version and then
> > supplements it for
> > C++.  When I use this, the above use of 'fabs' above results in an
> > C++overload
> > ambiguity error.
> >
> > It's easy to say "It's C++, use '<cmath>'",
>
> That wouldn't help. In namespace std, libc++ has always provided overloads
> resulting in the ambiguity you're now seeing (and FWIW, this is the
> overload set required by the C++ standard).
>
> [...]
> > and has odd incongruities such as:
> >
> > #include <math.h>
> >
> > float       (*pf)(float)        = std::fabs; // Okay, we have this
> > long double (*pld)(long double) = std::fabs; // Okay, we have this too
> > double      (*pd)(double)       = std::fabs; // Oops, no such function
>
> That seems a consequence of your (incorrect) patch. libc++'s current
> <math.h> doesn't provide any of these in namespace std, consistent with the
> requirements of the C++ standard. The intent is that no set of #includes
> gives you a partial overload set for any of these functions (except cases
> like 'abs', where C++ specifies that the overload set is split across
> multiple headers).
>
>
> The real issue is that libc++ doesn't support __fp16 (you should be able
> to observe this with both past versions of libc++ and with trunk if you
> change your testcase to use <cmath> and std::fabs). The right way to fix
> this would be to add __fp16 overloads to <math.h> (in the cases where the
> underlying compiler and language mode provide such a
> type) and ensure its __promote does the right thing for __fp16. It's up to
> the libc++ maintainers whether they consider this to be a regression and
> whether they'd accept a patch for it in the 3.8 release.
>
> But it's not a bug that libc++'s <math.h> now follows the C++ standard
> when compiling in C++ mode.
> --
> Richard
>
> _______________________________________________
> 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/20160123/1a56fd3d/attachment.html>