On Tue, 11 May 2021, Jonathan Wakely wrote: > On 11/05/21 11:16 -0400, Patrick Palka via Libstdc++ wrote: > > On Tue, 11 May 2021, Patrick Palka wrote: > > > > > floating_to_chars.cc includes the Ryu sources into an anonymous > > > namespace as a convenient way to give all its symbols internal linkage. > > > But an entity declared extern "C" always has external linkage, even > > > from within an anonymous namespace, so this trick doesn't work in the > > > presence of extern "C", and it causes the Ryu function generic_to_chars > > > to be visible from libstdc++.a. > > > > > > This patch removes the only use of extern "C" from our local copy of > > > Ryu, along with some declarations for never-defined functions that GCC > > > now warns about. > > > > > > Tested on x86_64-pc-linux-gnu, and also verified that generic_to_chars > > > is not visible from libstdc++.a. Does this look OK for trunk and the 11 > > > branch? > > > > Now with a testcase: > > > > -- >8 -- > > > > Subject: [PATCH] libstdc++: Remove extern "C" from Ryu sources > > > > floating_to_chars.cc includes the Ryu sources into an anonymous > > namespace as a convenient way to give all its symbols internal linkage. > > But an entity declared extern "C" always has external linkage, even > > from within an anonymous namespace, so this trick doesn't work in the > > presence of extern "C", and it causes the Ryu function generic_to_chars > > to be visible from libstdc++.a. > > > > This patch removes the only use of extern "C" from our local copy of > > Ryu along with some declarations for never-defined functions that GCC > > now warns about. > > > > Tested on x86_64-pc-linux-gnu, and also verified that generic_to_chars > > is not visible from libstdc++.a. Does this look OK for trunk and the 11 > > branch? > > > > libstdc++-v3/ChangeLog: > > > > * src/c++17/ryu/LOCAL_PATCHES: Update. > > * src/c++17/ryu/ryu_generic_128.h: Remove extern "C". > > Remove declarations for never-defined functions. > > * testsuite/20_util/to_chars/4.cc: New test. > > --- > > libstdc++-v3/src/c++17/ryu/LOCAL_PATCHES | 1 + > > libstdc++-v3/src/c++17/ryu/ryu_generic_128.h | 21 ++---------- > > libstdc++-v3/testsuite/20_util/to_chars/4.cc | 36 ++++++++++++++++++++ > > 3 files changed, 40 insertions(+), 18 deletions(-) > > create mode 100644 libstdc++-v3/testsuite/20_util/to_chars/4.cc > > > > diff --git a/libstdc++-v3/src/c++17/ryu/LOCAL_PATCHES > > b/libstdc++-v3/src/c++17/ryu/LOCAL_PATCHES > > index 51e504cb6ea..72ffad9662d 100644 > > --- a/libstdc++-v3/src/c++17/ryu/LOCAL_PATCHES > > +++ b/libstdc++-v3/src/c++17/ryu/LOCAL_PATCHES > > @@ -1,2 +1,3 @@ > > r11-6248 > > r11-7636 > > +r12-XXX > > diff --git a/libstdc++-v3/src/c++17/ryu/ryu_generic_128.h > > b/libstdc++-v3/src/c++17/ryu/ryu_generic_128.h > > index 2afbf274e11..6d988ab01eb 100644 > > --- a/libstdc++-v3/src/c++17/ryu/ryu_generic_128.h > > +++ b/libstdc++-v3/src/c++17/ryu/ryu_generic_128.h > > @@ -18,9 +18,9 @@ > > #define RYU_GENERIC_128_H > > > > > > -#ifdef __cplusplus > > -extern "C" { > > -#endif > > +// NOTE: These symbols are declared extern "C" upstream, but we don't want > > that > > +// because it'd override the internal linkage of the anonymous namespace > > into > > +// which this header is included. > > > > // This is a generic 128-bit implementation of float to shortest conversion > > // using the Ryu algorithm. It can handle any IEEE-compatible floating-point > > @@ -42,18 +42,6 @@ struct floating_decimal_128 { > > bool sign; > > }; > > > > -struct floating_decimal_128 float_to_fd128(float f); > > -struct floating_decimal_128 double_to_fd128(double d); > > - > > -// According to wikipedia (https://en.wikipedia.org/wiki/Long_double), this > > likely only works on > > -// x86 with specific compilers (clang?). May need an ifdef. > > -struct floating_decimal_128 long_double_to_fd128(long double d); > > - > > -// Converts the given binary floating point number to the shortest decimal > > floating point number > > -// that still accurately represents it. > > -struct floating_decimal_128 generic_binary_to_decimal( > > - const uint128_t bits, const uint32_t mantissaBits, const uint32_t > > exponentBits, const bool explicitLeadingBit); > > - > > // Converts the given decimal floating point number to a string, writing to > > result, and returning > > // the number characters written. Does not terminate the buffer with a 0. In > > the worst case, this > > // function can write up to 53 characters. > > @@ -63,8 +51,5 @@ struct floating_decimal_128 generic_binary_to_decimal( > > // = 1 + 39 + 1 + 1 + 1 + 10 = 53 > > int generic_to_chars(const struct floating_decimal_128 v, char* const > > result); > > > > -#ifdef __cplusplus > > -} > > -#endif > > > > #endif // RYU_GENERIC_128_H > > diff --git a/libstdc++-v3/testsuite/20_util/to_chars/4.cc > > b/libstdc++-v3/testsuite/20_util/to_chars/4.cc > > new file mode 100644 > > index 00000000000..96f6e5d010c > > --- /dev/null > > +++ b/libstdc++-v3/testsuite/20_util/to_chars/4.cc > > @@ -0,0 +1,36 @@ > > +// Copyright (C) 2021 Free Software Foundation, Inc. > > +// > > +// This file is part of the GNU ISO C++ Library. This library is free > > +// software; you can redistribute it and/or modify it under the > > +// terms of the GNU General Public License as published by the > > +// Free Software Foundation; either version 3, or (at your option) > > +// any later version. > > + > > +// This library is distributed in the hope that it will be useful, > > +// but WITHOUT ANY WARRANTY; without even the implied warranty of > > +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +// GNU General Public License for more details. > > + > > +// You should have received a copy of the GNU General Public License along > > +// with this library; see the file COPYING3. If not see > > +// <http://www.gnu.org/licenses/>. > > + > > +// { dg-do link { target c++17 } } > > +// { dg-require-effective-target ieee-floats } > > +// { dg-require-static-libstdcxx } > > +// { dg-additional-options "-static-libstdc++" } > > + > > +// Verify the Ryu symbol generic_to_chars doesn't inadvertently leak into > > +// libstdc++.a. If it did, this test would fail at link time with a > > multiple > > +// definition error. > > + > > +#include <charconv> > > + > > +extern "C" void generic_to_chars (void) { __builtin_abort(); } > > This test is "dg-do link" but that won't check whether this abort is > called. Did you mean to dg-do run?
I'm not sure if the abort call is necessary since the link step already fails with a multiple definition error (without the fix) even if the function is defined with an empty body. But since Jakub included an abort call in his testcase I carried it over :) Shall I just make it dg-do run, or perhaps keep it dg-do link and make the function body empty? Either seems to do the right thing. > > > > + > > +int > > +main() > > +{ > > + char x[64]; > > + std::to_chars(x, x+64, 42.L, std::chars_format::scientific); > > +} > > -- > > 2.31.1.527.g2d677e5b15 > > > >