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

[Richard Smith via cfe-dev]

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 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