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 [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 0D3783858C2B for ; Thu, 3 Nov 2022 16:30:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0D3783858C2B Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1667493015; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=zH9uLvCGAtQ+I/lDrZN/aJFVuBE7xpsb1RyT642Fd+8=; b=h2aV0t+wcW8U6b9jm7qu64boqWBshO04vgsonDwg1nwbwu2g5mQGtsn9c7y+PgXKOQ9AVg QVtdRfJrnKJqP3j0XZpzpHw5nfu7Z02fh8z3ur3plsg3IiHoPpkpPLALEE9NI2yopnZ/04 vKZ1cBUH+6Bj4mckZ/8amrxl6WNBb2o= Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-197-S4ZXgtOhOa6OTIJ6mKiZpQ-1; Thu, 03 Nov 2022 12:30:14 -0400 X-MC-Unique: S4ZXgtOhOa6OTIJ6mKiZpQ-1 Received: by mail-ed1-f69.google.com with SMTP id z9-20020a05640235c900b0046358415c4fso1737808edc.9 for ; Thu, 03 Nov 2022 09:30:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=zH9uLvCGAtQ+I/lDrZN/aJFVuBE7xpsb1RyT642Fd+8=; b=zCcQWHlW9iS33KKFREpnhY5iNkQf2Nnatgj0cFWnJEd9coyraGTC9OL/NBSw6AL5iq ESQGbfDuSIY18AqiQ4fiLZXmuI5jkRMDyK/pCOy5U24fErBIeKLd2Sw7irvbn0u/hRRv fz/L+R5jHXxsa/r5aRx4ob9YTjLLnudK9nkOlhAVDLbYKjHT6WwHgJRAHr5WieD7kIWF twjhUdWj4meocfG10ihy7UziaPHaHgZrURKNzU+dcepSTzFlquu5AB/LirEIiLCjTLwi n2QZrb1rsF1F+JZs/qa32Si3cDF10QVFNX6AkhfcESx4LW3330kguHmNLV0Qt3yoObCx xz5Q== X-Gm-Message-State: ACrzQf0zDJQt7C6Cd4CHjXnA3vkLSWtI7IVf0APdH1uU1Wp3PZ+gMXpA V/iNRb2ClDtWQIVU2m0rNwAv6T0M29hDc+JsYiD/azfmUn6v/+VGHXHYNaRxq2m2vUldHwnvaHq kutaFgXDdEmvKfYaPtthTVcfsKzapAyk= X-Received: by 2002:a17:907:2c74:b0:7a1:d333:f214 with SMTP id ib20-20020a1709072c7400b007a1d333f214mr30597014ejc.14.1667493012784; Thu, 03 Nov 2022 09:30:12 -0700 (PDT) X-Google-Smtp-Source: AMsMyM69EKrv7mGGT9oiP/498QjP0dGMkzmlfjLfj04NDrOVuGbmYHyIVa5MEoeI8T9NtfF+woyNxRsQ127scWnRZ/c= X-Received: by 2002:a17:907:2c74:b0:7a1:d333:f214 with SMTP id ib20-20020a1709072c7400b007a1d333f214mr30596993ejc.14.1667493012503; Thu, 03 Nov 2022 09:30:12 -0700 (PDT) MIME-Version: 1.0 References: <20221102134028.1032216-1-ppalka@redhat.com> In-Reply-To: <20221102134028.1032216-1-ppalka@redhat.com> From: Jonathan Wakely Date: Thu, 3 Nov 2022 16:30:01 +0000 Message-ID: Subject: Re: [PATCH] libstdc++: Declare const global variables inline To: Patrick Palka Cc: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-13.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, 2 Nov 2022 at 13:42, Patrick Palka via Libstdc++ wrote: > > IIUC such variables should be declared inline to avoid potential ODR > violations since they're otherwise considered to be distinct (internal > linkage) entities across TUs. > > The changes inside the regex_constants and execution namespace seem to > be unimplemented parts of P0607R0; the rest of the changes touch only > implementation details. > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? OK, thanks. My understanding was that [basic.def.odr] p14.5 means we don't get ODR violations, but I think that only works for sensible uses of the variables. Any inline function that takes the address of (or binds a reference to) one of these variables will get an ODR violation. So the patch is an improvement. > > libstdc++-v3/ChangeLog: > > * include/bits/atomic_wait.h (_detail::__platform_wait_alignment): > Declare inline. Remove redundant static specifier. > (__detail::__atomic_spin_count_relax): Declare inline. > (__detail::__atomic_spin_count): Likewise. > * include/bits/regex_automaton.h (__detail::_S_invalid_state_id): > Conditionally declare inline. Declare constexpr. Remove > redundant const and static specifiers. > * include/bits/regex_error.h (regex_constants::error_collate): Conditionally > declare inline. > (regex_constants::error_ctype): Likewise. > (regex_constants::error_escape): Likewise. > (regex_constants::error_backref): Likewise. > (regex_constants::error_brack): Likewise. > (regex_constants::error_paren): Likewise. > (regex_constants::error_brace): Likewise. > (regex_constants::error_badbrace): Likewise. > (regex_constants::error_range): Likewise. > (regex_constants::error_space): Likewise. > (regex_constants::error_badrepeat): Likewise. > (regex_constants::error_complexity): Likewise. > (regex_constants::error_stack): Likewise. > * include/ext/concurrence.h (__gnu_cxx::__default_lock_policy): > Likewise. Remove redundant static specifier. > * include/pstl/execution_defs.h (execution::seq): Conditionally declare > inline. > (execution::par): Likewise. > (execution::par_unseq): Likewise. > (execution::unseq): Likewise. > --- > libstdc++-v3/include/bits/atomic_wait.h | 8 +++---- > libstdc++-v3/include/bits/regex_automaton.h | 2 +- > libstdc++-v3/include/bits/regex_error.h | 26 ++++++++++----------- > libstdc++-v3/include/ext/concurrence.h | 2 +- > libstdc++-v3/include/pstl/execution_defs.h | 8 +++---- > 5 files changed, 23 insertions(+), 23 deletions(-) > > diff --git a/libstdc++-v3/include/bits/atomic_wait.h b/libstdc++-v3/include/bits/atomic_wait.h > index 76ed7409937..bd1ed56d157 100644 > --- a/libstdc++-v3/include/bits/atomic_wait.h > +++ b/libstdc++-v3/include/bits/atomic_wait.h > @@ -58,14 +58,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > #ifdef _GLIBCXX_HAVE_LINUX_FUTEX > #define _GLIBCXX_HAVE_PLATFORM_WAIT 1 > using __platform_wait_t = int; > - static constexpr size_t __platform_wait_alignment = 4; > + inline constexpr size_t __platform_wait_alignment = 4; > #else > // define _GLIBCX_HAVE_PLATFORM_WAIT and implement __platform_wait() > // and __platform_notify() if there is a more efficient primitive supported > // by the platform (e.g. __ulock_wait()/__ulock_wake()) which is better than > // a mutex/condvar based wait. > using __platform_wait_t = uint64_t; > - static constexpr size_t __platform_wait_alignment > + inline constexpr size_t __platform_wait_alignment > = __alignof__(__platform_wait_t); > #endif > } // namespace __detail > @@ -142,8 +142,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > #endif > } > > - constexpr auto __atomic_spin_count_relax = 12; > - constexpr auto __atomic_spin_count = 16; > + inline constexpr auto __atomic_spin_count_relax = 12; > + inline constexpr auto __atomic_spin_count = 16; > > struct __default_spin_policy > { > diff --git a/libstdc++-v3/include/bits/regex_automaton.h b/libstdc++-v3/include/bits/regex_automaton.h > index f95eb7dad6d..44bde42e212 100644 > --- a/libstdc++-v3/include/bits/regex_automaton.h > +++ b/libstdc++-v3/include/bits/regex_automaton.h > @@ -46,7 +46,7 @@ namespace __detail > */ > > typedef long _StateIdT; > - static const _StateIdT _S_invalid_state_id = -1; > + _GLIBCXX17_INLINE constexpr _StateIdT _S_invalid_state_id = -1; > > template > using _Matcher = std::function; > diff --git a/libstdc++-v3/include/bits/regex_error.h b/libstdc++-v3/include/bits/regex_error.h > index 74a1428c2c7..ab207650d44 100644 > --- a/libstdc++-v3/include/bits/regex_error.h > +++ b/libstdc++-v3/include/bits/regex_error.h > @@ -66,60 +66,60 @@ namespace regex_constants > }; > > /** The expression contained an invalid collating element name. */ > - constexpr error_type error_collate(_S_error_collate); > + _GLIBCXX17_INLINE constexpr error_type error_collate(_S_error_collate); > > /** The expression contained an invalid character class name. */ > - constexpr error_type error_ctype(_S_error_ctype); > + _GLIBCXX17_INLINE constexpr error_type error_ctype(_S_error_ctype); > > /** > * The expression contained an invalid escaped character, or a trailing > * escape. > */ > - constexpr error_type error_escape(_S_error_escape); > + _GLIBCXX17_INLINE constexpr error_type error_escape(_S_error_escape); > > /** The expression contained an invalid back reference. */ > - constexpr error_type error_backref(_S_error_backref); > + _GLIBCXX17_INLINE constexpr error_type error_backref(_S_error_backref); > > /** The expression contained mismatched [ and ]. */ > - constexpr error_type error_brack(_S_error_brack); > + _GLIBCXX17_INLINE constexpr error_type error_brack(_S_error_brack); > > /** The expression contained mismatched ( and ). */ > - constexpr error_type error_paren(_S_error_paren); > + _GLIBCXX17_INLINE constexpr error_type error_paren(_S_error_paren); > > /** The expression contained mismatched { and } */ > - constexpr error_type error_brace(_S_error_brace); > + _GLIBCXX17_INLINE constexpr error_type error_brace(_S_error_brace); > > /** The expression contained an invalid range in a {} expression. */ > - constexpr error_type error_badbrace(_S_error_badbrace); > + _GLIBCXX17_INLINE constexpr error_type error_badbrace(_S_error_badbrace); > > /** > * The expression contained an invalid character range, > * such as [b-a] in most encodings. > */ > - constexpr error_type error_range(_S_error_range); > + _GLIBCXX17_INLINE constexpr error_type error_range(_S_error_range); > > /** > * There was insufficient memory to convert the expression into a > * finite state machine. > */ > - constexpr error_type error_space(_S_error_space); > + _GLIBCXX17_INLINE constexpr error_type error_space(_S_error_space); > > /** > * One of *?+{ was not preceded by a valid regular expression. > */ > - constexpr error_type error_badrepeat(_S_error_badrepeat); > + _GLIBCXX17_INLINE constexpr error_type error_badrepeat(_S_error_badrepeat); > > /** > * The complexity of an attempted match against a regular expression > * exceeded a pre-set level. > */ > - constexpr error_type error_complexity(_S_error_complexity); > + _GLIBCXX17_INLINE constexpr error_type error_complexity(_S_error_complexity); > > /** > * There was insufficient memory to determine whether the > * regular expression could match the specified character sequence. > */ > - constexpr error_type error_stack(_S_error_stack); > + _GLIBCXX17_INLINE constexpr error_type error_stack(_S_error_stack); > > ///@} > } // namespace regex_constants > diff --git a/libstdc++-v3/include/ext/concurrence.h b/libstdc++-v3/include/ext/concurrence.h > index aea861b534f..7fd81490eff 100644 > --- a/libstdc++-v3/include/ext/concurrence.h > +++ b/libstdc++-v3/include/ext/concurrence.h > @@ -50,7 +50,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > // Compile time constant that indicates prefered locking policy in > // the current configuration. > - static const _Lock_policy __default_lock_policy = > + _GLIBCXX17_INLINE const _Lock_policy __default_lock_policy = > #ifndef __GTHREADS > _S_single; > #elif defined _GLIBCXX_HAVE_ATOMIC_LOCK_POLICY > diff --git a/libstdc++-v3/include/pstl/execution_defs.h b/libstdc++-v3/include/pstl/execution_defs.h > index 13b002931e8..3eca558dac2 100644 > --- a/libstdc++-v3/include/pstl/execution_defs.h > +++ b/libstdc++-v3/include/pstl/execution_defs.h > @@ -107,10 +107,10 @@ class unsequenced_policy > }; > > // 2.8, Execution policy objects > -constexpr sequenced_policy seq{}; > -constexpr parallel_policy par{}; > -constexpr parallel_unsequenced_policy par_unseq{}; > -constexpr unsequenced_policy unseq{}; > +_GLIBCXX17_INLINE constexpr sequenced_policy seq{}; > +_GLIBCXX17_INLINE constexpr parallel_policy par{}; > +_GLIBCXX17_INLINE constexpr parallel_unsequenced_policy par_unseq{}; > +_GLIBCXX17_INLINE constexpr unsequenced_policy unseq{}; > > // 2.3, Execution policy type trait > template > -- > 2.38.1.381.gc03801e19c >