From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x635.google.com (mail-ej1-x635.google.com [IPv6:2a00:1450:4864:20::635]) by sourceware.org (Postfix) with ESMTPS id BDBEC3857B92; Fri, 2 Sep 2022 14:10:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BDBEC3857B92 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ej1-x635.google.com with SMTP id p16so4105757ejb.9; Fri, 02 Sep 2022 07:10:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=/EdHK4MpO3f9ht8p2DWNSSo/bl6+BssKmGJ4suYzNO4=; b=Ap+DBT/uTdOLDIWZOn/ZXscaVLlu0IQ7NWzn2q52Iwx5XTzmFSKIoUCnvIf61ulTaB N+Zzi2gn3jsoYzqOQIGJ8bBszZiwGLcs+878bjXhKK8zvxH/WkokpqIwHm0ENj2GI/BG VeUXQj9fAi2czm0xtfq1EG3eej4VjqkNTALvAbJPtMsETQgTPL3F9CLtE8VoTUxWN3Gk d1HfJe7915ulZut7rBB+BSYaLJHjiBxim0QxuZZXk+o8QwDO9Piyld/65EaPcoArUvKN V3dC1a108BW98qWFfuqvUzuhtNrjzU/yelsxqHM4u3MyakbLBGLVwYb01ggmIBzQXdue Z/xw== 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; bh=/EdHK4MpO3f9ht8p2DWNSSo/bl6+BssKmGJ4suYzNO4=; b=XmPzx1daNmBlrdz0sb+gHDuNgWq46qpolUiQeKHEvzi/OVeR9fgYFooxQdhhIx5JT/ 5Zze7HWw4HKqmGIQxJ6owsuinP0N8+p2anF6aqrhgJheDDBv5YYq0iMXNzM3ICcHw2/t cWULpk+pxd33OxnER09j89ox+ue56jVxVGMjkTSpVo8hcoXZ2AKzFEGu2rp9e+GVG3Fy iI5u/oPth+8LYsxs2SWGcv7HdtuXOgvsyBeVkLDxW5+Y7LWg+SUYbhGp+DvHhQnRaQ12 gEcZzO+Go1c3G/GPDuE8gkIzIlW6vf5lRG3PW0eXejuuAIsbmHNZkM0esLxr39GeqxCK HL1Q== X-Gm-Message-State: ACgBeo0MGby3CaCSbMOMpfJiON9lpy5AaEk3pOe4uIo85+zjMwSki1K7 BNOXJ2N7gxP2B23ftAIaJ6IZ4EofNHcaXXESbrM= X-Google-Smtp-Source: AA6agR6ZwpX8oHrIWbOk+apxi8HRfIbOxXi7nAg2igEvaMsqcOm3NzaIcdyMKsvKVEFmM1t7QTvROH6AJIo8asx4RkQ= X-Received: by 2002:a17:907:a406:b0:741:8657:c463 with SMTP id sg6-20020a170907a40600b007418657c463mr17717552ejc.386.1662127841282; Fri, 02 Sep 2022 07:10:41 -0700 (PDT) MIME-Version: 1.0 References: <20220902131826.697225-1-ppalka@redhat.com> <454f13fd-03dd-7d5a-895d-61a81e7f7773@idea> In-Reply-To: <454f13fd-03dd-7d5a-895d-61a81e7f7773@idea> From: Jonathan Wakely Date: Fri, 2 Sep 2022 15:10:29 +0100 Message-ID: Subject: Re: [PATCH] libstdc++: Fix laziness of __and/or/not_ To: Patrick Palka Cc: "libstdc++" , gcc-patches Content-Type: multipart/alternative; boundary="0000000000008d114605e7b24b85" X-Spam-Status: No, score=-6.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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: --0000000000008d114605e7b24b85 Content-Type: text/plain; charset="UTF-8" On Fri, 2 Sep 2022, 14:35 Patrick Palka via Libstdc++, < libstdc++@gcc.gnu.org> wrote: > On Fri, 2 Sep 2022, Patrick Palka wrote: > > > r13-2230-g390f94eee1ae69 redefined the internal logical operator traits > > __and_, __or_ and __not as alias templates that directly resolve to > > true_type or false_type. But it turns out using an alias template here > > causes the traits to be less lazy than before because we now compute > > the logical result immediately upon _specialization_ of the trait, and > > not later upon _completion_ of the specialization. > > > > Thus, for example, in > > > > using type = __and_>; > > > > we now compute the conjunction and thus instantiate A even though we're > > in a context that doesn't require completion of the __and_. What's > > worse is that we now compute the negation and thus instantiate B as well > > (for the same reason), independent of the __and_ and the value of A! > > Thus the traits are now less lazy and composable than before. > Ah good catch. > > > Fortunately, the fix is cheap and simple: redefine these traits as class > > templates instead of as alias templates so that completion not > > specialization triggers computation of the logical result. I added > > comprehensive short circuiting tests for these internal logical operator > > traits in short_circuit.cc guarded by __GLIBCXX__, not sure if > > that's the best place for them. (Note that before this fix, assert #5 > > and #10 guarded by __GLIBCXX__ would induce a hard error due to this > > bug). > I don't bother guarding libstdc++-specific checks, because LLVM and MSVC folk are allergic to anything that's been anywhere near GPL code. But doing so is a kindness for any users who do decide to use our tests. Maybe I should go through tests where I've added a comment saying "GCC extension" and guard them this way > FWIW this change doesn't seem to have a measurable compile time/memory > impact on the stress test from r13-2230. For std/ranges/adaptors/join.cc, > memory usage increases by around 1% and compile time decreases by around > 1%. > Great. > > > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? > OK, thanks > > > libstdc++-v3/ChangeLog: > > > > * include/std/type_traits (__or_, __and_, __not_): Redefine as a > > class template instead of an alias template. > > * testsuite/20_util/logical_traits/requirements/short_circuit.cc: > > Add more tests for conjunction and disjunction. Add corresponding > > tests for __and_ and __or_v. > > --- > > libstdc++-v3/include/std/type_traits | 12 ++++++-- > > .../requirements/short_circuit.cc | 29 +++++++++++++++++++ > > 2 files changed, 38 insertions(+), 3 deletions(-) > > > > diff --git a/libstdc++-v3/include/std/type_traits > b/libstdc++-v3/include/std/type_traits > > index 615791f29c8..2feb4b145c5 100644 > > --- a/libstdc++-v3/include/std/type_traits > > +++ b/libstdc++-v3/include/std/type_traits > > @@ -168,13 +168,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > // to either true_type or false_type which allows for a more efficient > > // implementation that avoids recursive class template instantiation. > > template > > - using __or_ = decltype(__detail::__or_fn<_Bn...>(0)); > > + struct __or_ > > + : decltype(__detail::__or_fn<_Bn...>(0)) > > + { }; > > > > template > > - using __and_ = decltype(__detail::__and_fn<_Bn...>(0)); > > + struct __and_ > > + : decltype(__detail::__and_fn<_Bn...>(0)) > > + { }; > > > > template > > - using __not_ = __bool_constant; > > + struct __not_ > > + : __bool_constant > > + { }; > > /// @endcond > > > > #if __cplusplus >= 201703L > > diff --git > a/libstdc++-v3/testsuite/20_util/logical_traits/requirements/short_circuit.cc > b/libstdc++-v3/testsuite/20_util/logical_traits/requirements/short_circuit.cc > > index 86996b27fa5..ff90f8a47c3 100644 > > --- > a/libstdc++-v3/testsuite/20_util/logical_traits/requirements/short_circuit.cc > > +++ > b/libstdc++-v3/testsuite/20_util/logical_traits/requirements/short_circuit.cc > > @@ -14,6 +14,10 @@ static_assert(!std::conjunction_v invalid>); > > static_assert(!std::conjunction_v); > > static_assert(!std::conjunction_v invalid>); > > static_assert(!std::conjunction_v invalid, invalid>); > > +static_assert(!std::conjunction_v > + std::conjunction, > > + std::disjunction, > > + std::negation>); > > > > // [meta.logical]/8: For a specialization disjunction, if > > // there is a template type argument B_i for which bool(B_i::value) is > true, > > @@ -24,3 +28,28 @@ static_assert(std::disjunction_v invalid>); > > static_assert(std::disjunction_v); > > static_assert(std::disjunction_v invalid>); > > static_assert(std::disjunction_v invalid, invalid>); > > +static_assert(std::disjunction_v > + std::conjunction, > > + std::disjunction, > > + std::negation>); > > + > > +#if __GLIBCXX__ > > +// Also test the corresponding internal traits __and_, __or_ and __not_. > > +static_assert(!std::__and_v); > > +static_assert(!std::__and_v); > > +static_assert(!std::__and_v); > > +static_assert(!std::__and_v invalid>); > > +static_assert(!std::__and_v > + std::__and_, > > + std::__or_, > > + std::__not_>); > > + > > +static_assert(std::__or_v); > > +static_assert(std::__or_v); > > +static_assert(std::__or_v); > > +static_assert(std::__or_v invalid>); > > +static_assert(std::__or_v > + std::__and_, > > + std::__or_, > > + std::__not_>); > > +#endif > > -- > > 2.37.2.490.g6c8e4ee870 > > > > > > --0000000000008d114605e7b24b85--