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

Tue Feb 2 02:29:47 PST 2016

Sorry for the delay getting back to this.  I am attaching a revised patch (with respect to #259486) that addresses the issue that James commented on.  The test case:

test/std/depr/depr.c.headers/math_h.pass.cpp

fails with these changes, but this is because the test is expecting the names in ‘<math.h>’ to be overloaded in the global namespace.  I also made some additional changes to ‘<stdio.h>’ to wrap the macros for ‘getc’, ‘putc’, etc. using the same pattern that is used to achieve the same task in ‘<math.h>’.  The ‘#undef’s for these was causing link failures for against our C library which does not provide callable functions for these and instead uses the macros provided by Newlib.  I liked the pattern used in ‘<math.h>’ and thought that it neatly addressed the requirements of C++.  Similar changes are probably a good idea for ‘<ctype.h>’ and ‘<wctype.h>’, but I did not implement these.

Thanks,

            MartinO

From: Martin J. O'Riordan [mailto:martin.oriordan at movidius.com] 
Sent: 27 January 2016 15:41
To: 'James Y Knight'
Cc: 'David Chisnall'; 'Clang Dev'
Subject: RE: [cfe-dev] LibC++ v3.8 - Problems with ISO C wrapper headers

Hmm!  In order for it to matter, the program would have to do:

    #include <math.h>

    #include <cmath>

which I think is unlikely, but:

    #include <math.h>

    #include “otherfile.h”

where ‘otherfile.h’ then includes ‘<cmath>’, which is more likely to happen when ‘<cmath>’ is included by proxy after ‘<math.h>’.  But you are correct, I hadn’t thought of this and the pattern would need to be refined to address this.  I did not come across this scenario in the LibC++ test-suite or my own.

Thanks,

           MartinO

From: James Y Knight [mailto:jyknight at google.com] 
Sent: 27 January 2016 15:29
To: Martin J. O'Riordan
Cc: David Chisnall; Clang Dev
Subject: Re: [cfe-dev] LibC++ v3.8 - Problems with ISO C wrapper headers

This doesn't seem right -- won't it break code that does this:

#include <math.h>

#include <cmath>

? (since the "#ifdef _LIBCPP_INCLUDING_STDC_HEADER" is within the "#ifndef _LIBCPP_MATH_H" block, and thus only gets checked once)

On Wed, Jan 27, 2016 at 10:22 AM, Martin J. O'Riordan via cfe-dev <cfe-dev at lists.llvm.org> wrote:

I suppose I am more focussed on embedded systems than hosted - thus the reference to 'newlib'; and I'm sure that there are many alternative STDC libraries that I have never heard of.  But I think that the LibC++ wrapper headers is still a good place to abstract such portability issues so that users of LibC++ are unaware of the ISO C header implementation that lies beneath.

I have also attached a patch file computed against the #258931 revision on:

     https://llvm.org/svn/llvm-project/libcxx/branches/release_38/include

Although many files have changed, the nature of the changes is quite simple.

  o  In each of the '<cXXXX>' files I have inserted:

       #define _LIBCPP_INCLUDING_STDC_HEADER

     before each '#include <####.h>' ISO C header, and followed by:

       #undef _LIBCPP_INCLUDING_STDC_HEADER

     I have done this for all LibC++ headers that include an ISO C header even if
     it does not refer to a LibC++ wrapping header, in case one C header
     includes another in any particular ISO C header implementation.

  o  In each of the '<XXXX.h>' ISO C header wrappers, where appropriate I have
     replaced:

       #ifdef __cplusplus
       ...
       #endif // __cplusplus

     with:

       #ifdef _LIBCPP_INCLUDING_STDC_HEADER
       ...
       #endif // _LIBCPP_INCLUDING_STDC_HEADER

  o  Before the C++ overloaded functions are introduced at global scope, I have
     inserted:

       _LIBCPP_BEGIN_NAMESPACE_STD

     and afterwards:

       _LIBCPP_END_NAMESPACE_STD

  o  Finally, in '<math.h>' in particular, I have prefixed the forwarded
     names with '::' since at this point in the 'using ::name;' has not yet
     been seen.

The patch is a suggestion, and the '_LIBCPP_INCLUDING_STDC_HEADER' name I picked are just my idea for following the existing convention used by the LibC++ maintainers.

        MartinO

-----Original Message-----
From: Dr D. Chisnall [mailto:dc552 at hermes.cam.ac.uk] On Behalf Of David Chisnall
Sent: 27 January 2016 9:25
To: Martin.ORiordan at Movidius.com
Cc: Craig, Ben; Clang Dev
Subject: Re: [cfe-dev] LibC++ v3.8 - Problems with ISO C wrapper headers

On 26 Jan 2016, at 20:09, Martin J. O'Riordan via cfe-dev <cfe-dev at lists.llvm.org> wrote:
>
> And C++ also requires that some parts of the interfaces from C are presented to C++ as functions - examples being ‘fpclassify’, ‘signbit’ etc.  These have to be handled differently, and I think that the current LibC++ approach to these is good and maintains semantic compatibility with C.
>
> I had intended to build a patch file of my changes today in case anyone is interested, but other things got me busy.  I’ll do that tomorrow and attach it to this thread.
>
> I think that it is a good idea to have LibC++ provide its own wrapper headers for the C headers.  It is a logical place to deal with portability issues that arise when referring to C headers provided by newlib, uclibc or glibc (and other less well known implementations).  The handling of these portability issues will mean that the ‘c’ prefixed C++ headers will need little or no alteration and just maintain the ‘std’ namespace.
>

I note that none of your listed libc implementations are the defaults on the operating systems that ship libc++ as the default C++ standard library implementation…

It would be very helpful for people who are working on this to provide a set of recommendations for C library maintainers to make C++ interoperability easier.  That way, those of us who do have control over the libc headers can work from the other end to improve the situation.

David

_______________________________________________
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/20160202/c367e945/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: stdc-wrappers.patch
Type: application/octet-stream
Size: 31767 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160202/c367e945/attachment.obj>