This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Ping^4 Define __CORRECT_ISO_CPP_STRING_H_PROTO correctly for Clang.


Joseph, are you ok with this? This is simple addition that could go to 2.19,
is Brooks rationale sufficient for you?

On Thu, Dec 19, 2013 at 10:12:46AM -0800, Brooks Moses wrote:
> Ping^4?
> 
> On Wed, Dec 11, 2013 at 6:02 PM, Brooks Moses <bmoses@google.com> wrote:
> > Ping^3.
> >
> > In the string/string.h and string/strings.h headers, we have a couple
> > of macros that "tell the caller that we provide correct C++
> > prototypes" according to the comment; they are used to determine
> > whether to wrap some prototypes in "extern "C++"" (and provide
> > multiple overloads of them, and some other magic) when __cplusplus is
> > defined.
> >
> > The macros are set to check for sufficiently-recent GCC versions (4.4
> > and later), but this is not the right check for non-GCC compilers.  In
> > particular, these macros should also be set when using Clang -- if
> > they are not set, then Clang will be unable to correctly diagnose a
> > number of subtle bugs that will be errors in GCC compilations.
> >
> > As per discussion on earlier versions of this patch, rather than
> > restrict the fix to Clang per se, we assume that all C++ compilers that
> > claim to fully support C++98 are using a standard-conforming C++
> > standard library, which seems pretty reasonable.  Clang has been
> > providing an appropriate value of __cplusplus since May 2012.
> >
> >
> > As it's been a while since my last ping, here's the previous discussion
> > of why this is the best approach:
> >
> > On Mon, Sep 9, 2013 at 3:00 PM, Brooks Moses <bmoses@google.com> wrote:
> >> On Thu, Aug 29, 2013 at 5:18 AM, Joseph S. Myers
> >> <joseph@codesourcery.com> wrote:
> >>> What's the logic behind the GCC version check in the first place?  Is it
> >>> about compiler (language) features, or about corresponding support in the
> >>> standard C++ library?  If the former, do you need a Clang version check?
> >>> If the latter, do you need a check on what version of libstdc++ or libc++
> >>> might be used with Clang?
> >>
> >> This appears to be about corresponding support in libstdc++.  The
> >> original patch was actually only posted to gcc-patches@, here [1]:
> >>  http://gcc.gnu.org/ml/gcc-patches/2009-01/msg01457.html
> >> Prior to the patch, libstdc++'s cstring provided the C++ declarations
> >> of the string.h functions.  After the patch, the libstdc++ cstring
> >> only provides the C++ string.h declarations if these macros are not
> >> defined, so as to avoid conflicting with glibc's string.h
> >> declarations.
> >>
> >> In libc++, the corresponding declarations are guarded by a simple
> >> "!defined(__GLIBC__)" check, and have been since the very early days
> >> of the project; thus this should be safe for that as well [2].
> >>
> >> In principle, the correct solution might be replacing the GCC version
> >> check with a check on __GLIBCXX__ and _LIBCPP_VERSION.  However,
> >> __GLIBCXX__ is not monotonic with the GCC version[3] so this would be
> >> a very messy check, and there's the issue of getting the __GLIBCXX__
> >> value in a way that is guaranteed not to introduce circular header
> >> dependencies in libstdc++ or any other C++ library.  So unfortunately
> >> I don't think that's workable in practice.
> >>
> >> I brought this up with the Clang development mailing list, and the
> >> answer I got there was that "In practice, any of the solutions is
> >> probably fine: glibc gets upgraded along with the system, which
> >> implies a newer gcc, which implies clang will find a newer libstdc++"
> >> [4].
> >>
> >> Thus, I'd like to go with Andrew Pinski's idea of using a
> >> "__cplusplus>=199711L" check, and use the legacy behavior (check for
> >> GCC of 4.4 or later) otherwise.  As I explain in the code comment,
> >> this basically assumes that C++ compilers that claim to fully support
> >> C++98 are using a standard-conforming C++ standard library, which
> >> seems pretty reasonable.  Clang has been providing an appropriate
> >> value of __cplusplus since May of last year.
> >>
> >> The revised patch is attached.  I've tested it by compiling a file
> >> that includes the headers and checks the macro values, with GCC 4.6,
> >> GCC 4.8, and a recent build of Clang, and I confirmed that the macros
> >> are defined as expected in all cases.  I've attached the test program
> >> as well.
> >>
> >> Is this version ok to commit?
> >>
> >> Thanks,
> >> - Brooks
> >>
> >>
> >> [1] For archival purposes: There was also a bugfix on it sent to libc-hacker@:
> >> http://sourceware.org/ml/libc-hacker/2009-01/msg00013.html
> >>
> >> [2]: Discussion of the libc++ cstring prototypes here:
> >> http://llvm.org/svn/llvm-project/libcxx/trunk/include/cstring
> >> http://llvm.org/bugs/show_bug.cgi?id=7983
> >>
> >> [3]: See list of __GLIBCXX__ values over the years:
> >> http://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html
> >>
> >> [4] This quote is from email from Eli Friedman:
> >> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-August/031657.html
> >>
> >> [5] Clang generally followed GCC practice for __cplusplus values;
> >> discussion and patch here:
> >> http://clang-developers.42468.n3.nabble.com/Value-of-cplusplus-in-GNU-modes-td3957527.html
> >> http://llvm.org/viewvc/llvm-project?revision=156113&view=revision
> >
> >
> > ---
> > 2013-09-09  Brooks Moses  <bmoses@google.com>
> >
> >         * string/string.h (__CORRECT_ISO_CPP_STRING_H_PROTO): Define for
> >         all compilers that claim C++98 compliance, not just GCC.
> >         * string/strings.h (__CORRECT_ISO_CPP_STRINGS_H_PROTO):
> >         Likewise.
> >
> >  string/string.h  | 8 ++++++--
> >  string/strings.h | 8 ++++++--
> >  2 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/string/string.h b/string/string.h
> > index ecc3fef..3e66674 100644
> > --- a/string/string.h
> > +++ b/string/string.h
> > @@ -31,8 +31,12 @@ __BEGIN_DECLS
> >  #define        __need_NULL
> >  #include <stddef.h>
> >
> > -/* Tell the caller that we provide correct C++ prototypes.  */
> > -#if defined __cplusplus && __GNUC_PREREQ (4, 4)
> > +/* Provide correct C++ prototypes, and indicate this to the caller.  This
> > +   requires a compatible C++ standard library.  As a heuristic, we provide
> > +   these when the compiler indicates full conformance with C++98 or later,
> > +   and for older GCC versions that are known to provide a compatible
> > +   libstdc++.  */
> > +#if __cplusplus >= 199711L || (defined __cplusplus && __GNUC_PREREQ (4, 4))
> >  # define __CORRECT_ISO_CPP_STRING_H_PROTO
> >  #endif
> >
> > diff --git a/string/strings.h b/string/strings.h
> > index dad0a04..190147b 100644
> > --- a/string/strings.h
> > +++ b/string/strings.h
> > @@ -27,8 +27,12 @@
> >  # define __need_size_t
> >  # include <stddef.h>
> >
> > -/* Tell the caller that we provide correct C++ prototypes.  */
> > -# if defined __cplusplus && __GNUC_PREREQ (4, 4)
> > +/* Provide correct C++ prototypes, and indicate this to the caller.  This
> > +   requires a compatible C++ standard library.  As a heuristic, we provide
> > +   these when the compiler indicates full conformance with C++98 or later,
> > +   and for older GCC versions that are known to provide a compatible
> > +   libstdc++.  */
> > +# if __cplusplus >= 199711L || (defined __cplusplus && __GNUC_PREREQ (4, 4))
> >  #  define __CORRECT_ISO_CPP_STRINGS_H_PROTO
> >  # endif
> >
> > --
> > 1.8.5.1
> >


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]