From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 5543D398B875 for ; Tue, 11 May 2021 17:04:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5543D398B875 Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-279-9KbFUGYcM9KaAoEzGAx7Ng-1; Tue, 11 May 2021 13:04:31 -0400 X-MC-Unique: 9KbFUGYcM9KaAoEzGAx7Ng-1 Received: by mail-qv1-f69.google.com with SMTP id t1-20020a0ca6810000b029019e892416e6so16068046qva.9 for ; Tue, 11 May 2021 10:04:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:date:to:cc:subject:in-reply-to:message-id :references:mime-version; bh=WD8XXYgzTH8g9iujqapjPTlBnV7KiYJWb3a4G4TW/kA=; b=VWVZEK0HtFsMBkit0K4cPP7s0DW9mgHfCCLfycZxtO8bUssCjqy5dCcT661B1yTb8l aj3MDlP0r9fdnHIMK3RngNzR9RTk6mTSQyFT7gs1JGWLm275rpuwhHaRBh6Mtlj7wubU c+XU58oqvG9t3AMU+zDbfQbVYK4JQ8e6OKAesxDu8jglnLXk5VqkukMDiBctaS+XxekP 9N6ddlEQuo5e5M1xBzmi/sodGpkOEJoTX87r5yPXryoWyYhiBVNJYYpDMt2ynAm++Mmh ZlamwUTYN3iVrtOqZEspJJjUYBlQCJE9/6DNTAZweElNOlc1xvRz4W3HYi88QEd4Lvi6 ZyeA== X-Gm-Message-State: AOAM530KcfGEsK8l4YGSeWfV7L7ivWOFse3FFY5n5YfMh6VrPAsxRs3X mqDczTthgulVfq5OakydoB5l6S2OO46g5usHnvESZXEjJu9GKeNCW6D/2rDFmfl8MilCSWoipaH RR7GQH52L9rjXJxKNfQ== X-Received: by 2002:a0c:f0c9:: with SMTP id d9mr30813636qvl.3.1620752670435; Tue, 11 May 2021 10:04:30 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwzW46d6UYzxV4z3Cxh4mQ1F0EgAqMmsJKdMHHexfqsI2TNXwD3jENN3wsh4iXN+rPuGUeTOw== X-Received: by 2002:a0c:f0c9:: with SMTP id d9mr30813606qvl.3.1620752670091; Tue, 11 May 2021 10:04:30 -0700 (PDT) Received: from [192.168.1.130] (ool-457d493a.dyn.optonline.net. [69.125.73.58]) by smtp.gmail.com with ESMTPSA id j28sm7115210qkl.35.2021.05.11.10.04.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 May 2021 10:04:29 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Tue, 11 May 2021 13:04:28 -0400 (EDT) To: Jonathan Wakely cc: Patrick Palka , libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: Re: [PATCH] libstdc++: Remove extern "C" from Ryu sources In-Reply-To: <20210511165125.GX3008@redhat.com> Message-ID: <6bdc4ee6-522e-1ccf-3318-1c18e18728ca@idea> References: <20210511142427.221984-1-ppalka@redhat.com> <75e0367d-4fc-4de5-c96b-fe839cf547@idea> <20210511165125.GX3008@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-15.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=unavailable autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 May 2021 17:04:35 -0000 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 > > +// . > > + > > +// { 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 > > + > > +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 > > > >