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.133.124]) by sourceware.org (Postfix) with ESMTPS id D55BD3858C2C for ; Thu, 1 Jun 2023 20:31:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D55BD3858C2C 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=1685651462; 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=o7aFita2KiWoxFlSqto9r3rNSwjHTz6+2FFZ02GtTTY=; b=VaUBhtb1jceoOXt8VKw3i5zYbIvZvi+iDximlpRFAHW1BpiRswkqZNzWn41ak4+yVgD/tl 0Nvg4l6s1Ocq8RsQdjQLoR9Pim6wNpUC9FEbeMmkRGaeJRcdFqUsV2YPDmXx5XRKZXOm+m npeLELGJpIzULKAWeQRhKwNcVVF9zXU= Received: from mail-lj1-f198.google.com (mail-lj1-f198.google.com [209.85.208.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-54-arNJoDh4P6OAW554uVzsNw-1; Thu, 01 Jun 2023 16:31:01 -0400 X-MC-Unique: arNJoDh4P6OAW554uVzsNw-1 Received: by mail-lj1-f198.google.com with SMTP id 38308e7fff4ca-2b15a327c83so10156751fa.1 for ; Thu, 01 Jun 2023 13:31:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685651459; x=1688243459; 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=o7aFita2KiWoxFlSqto9r3rNSwjHTz6+2FFZ02GtTTY=; b=k43/8bSEjmariHD90BmFVYiR+9gMg2cKIimvLKpxHL+mnfNsbDjaYq0KfrqlG02Z8X u0HkXEJoYcnVdg7NCfvRY8TmbBHc2tMPn+rgD9FK6v8CwEqcvy7r5ahmbzqc/k4bfmiR QcvVOH/TtZavLH9Pw5bgpIeac5nJEqHA68/y+4kiIzLj3BF2Wwmi3i+do0uoOF/8CgDC o9Luf7I2wIloaWPhmrtqhJplsQuFNmAPzJdu3EQHswG6QedhlEkMV5V506J1c9Z77/bY EiNXy90m9SvNjUaP5+UTxvnuTwgvSJM8bN5GHDVKcUg4JxUcYxehLp80keWQUCo4SouO 2m5A== X-Gm-Message-State: AC+VfDxJUigGWkIPnU9AAPyNaNVEhJg+1reAvMYChG9uAHU3aHNJVlE7 WQ0alhNbYCfXRivDDcgRxSwmG3oCwX5eyw0rwL0jNzmUbvutJQG5J/Ahn53qirZdrNrs+SdAssD sCLFr2hlY7A7wFbtdlyuUXbN6uwAGT8+5YWIqMmY8xA== X-Received: by 2002:a2e:9e51:0:b0:2af:20c9:3f8a with SMTP id g17-20020a2e9e51000000b002af20c93f8amr364768ljk.7.1685651459750; Thu, 01 Jun 2023 13:30:59 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6kVLtsV8FXyZeKLdavJO9puWrQiJUv7Vo/LRQgP6tApnB+i5a5JrRuNHWMlZ9E+8/VpbmzscNIXaqLao1Pbn8= X-Received: by 2002:a2e:9e51:0:b0:2af:20c9:3f8a with SMTP id g17-20020a2e9e51000000b002af20c93f8amr364761ljk.7.1685651459297; Thu, 01 Jun 2023 13:30:59 -0700 (PDT) MIME-Version: 1.0 References: <20230429101640.1697750-1-arsen@aarsen.me> <20230429101640.1697750-3-arsen@aarsen.me> In-Reply-To: <20230429101640.1697750-3-arsen@aarsen.me> From: Jonathan Wakely Date: Thu, 1 Jun 2023 21:30:47 +0100 Message-ID: Subject: Re: [PATCH 2/2] libstdc++: Replace all manual FTM definitions and use To: =?UTF-8?Q?Arsen_Arsenovi=C4=87?= Cc: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="000000000000726a0d05fd17502e" X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: --000000000000726a0d05fd17502e Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sat, 29 Apr 2023 at 11:24, Arsen Arsenovi=C4=87 via Libstdc++ < libstdc++@gcc.gnu.org> wrote: > libstdc++-v3/ChangeLog: > > * libsupc++/typeinfo: Switch to bits/version.h for > __cpp_lib_constexpr_typeinfo. > > Does this change have an impact on compilation speed? With this change we'll be re-including bits/version.h multiple times in most compilations, and unlike other headers the preprocessor can't optimize away the second and subsequent times its' included, because the header isn't idempotent. It will only affect the preprocessing phase, which is a fraction of the time taken by template instantiation and middle end optimizations, but I'd like to know it's not *too* expensive before committing to this approach. > @@ -234,9 +234,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > return __atomic_test_and_set (&_M_i, int(__m)); > } > > -#if __cplusplus > 201703L > -#define __cpp_lib_atomic_flag_test 201907L > - > +#ifdef __cpp_lib_atomic_flag_test > _GLIBCXX_ALWAYS_INLINE bool > test(memory_order __m =3D memory_order_seq_cst) const noexcept > { > This is more "structured" and maintainable than the current ad-hoc way we deal with FTMs, but this seems like a readability/usability regression in terms of being able to open the header and see "ah this feature is only available for C++20 and up". Instead you can see it's available for the specified FTM, but now you have to go and find where that's defined, and that's not even defined in C++, it's in the version.def file. It's also defined in bits/version.h, but that's a generated file and so is very verbose and long. diff --git a/libstdc++-v3/include/bits/move_only_function.h > b/libstdc++-v3/include/bits/move_only_function.h > index 71d52074978..81d7d9f7c0a 100644 > --- a/libstdc++-v3/include/bits/move_only_function.h > +++ b/libstdc++-v3/include/bits/move_only_function.h > @@ -32,7 +32,10 @@ > > #pragma GCC system_header > > -#if __cplusplus > 202002L > +#define __glibcxx_want_move_only_function > +#include > + > +#ifdef __cpp_lib_move_only_function > Here's another case where I think the __cplusplus > 202002L is more discoverable. Although maybe I'm biased, because I look at that and immediately see "C++23 and up". Maybe the average user finds that less clear. Maybe the average user doesn't need to look at this anyway, but I know *I* do it fairly often. I wonder if it would help if we kept a comment there with a (possibly imprecise) hint about the conditions under which the feature is defined. So in this case: // Only defined for C++23 #ifdef __cpp_lib_move_only_function That retains the info that's currently there, and is even more readable than the __cplusplus check. There's a risk that those comments would get out of step with reality, which is one of the things this patch set aims to solve. But I think in practice that's unlikely. std::move_only_function isn't suddenly going to become available in C++20, or stop being available in C++23 and move to C++26. What do you think? --000000000000726a0d05fd17502e--