From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2303 invoked by alias); 30 Sep 2019 20:21:46 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 2091 invoked by uid 89); 30 Sep 2019 20:21:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mail-wr1-f66.google.com Received: from mail-wr1-f66.google.com (HELO mail-wr1-f66.google.com) (209.85.221.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Sep 2019 20:21:44 +0000 Received: by mail-wr1-f66.google.com with SMTP id r3so12816781wrj.6; Mon, 30 Sep 2019 13:21:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=nDOOMPT2r75vznEqQmcATNuBdtAVOWiW6IJkRLMBD7k=; b=knZjgYfl0DmuJqtazgOElkmSdRBCwm/EVcacbEeK1Ko4v458TXQx/ckw8zVPT3So3G Ju6O6Ro4757HRGGjast9WxW8DfqZy2B74NVcU2vHNt+wTVrSI/iyyIJbx0fOyi25CJrS v4I6HE/iXtHWrCzr7tkXkdMq7GzKzOwQym7zMG7z6vwk9JR+eR2Jf1/5xzPWdPJ3BquU 8ozdYRgRZnAO0o1u90OseVhUXbNAWZ2nN5aGzR4zJv/gAnk9Q8995zPtCd3pV+V+xXcI wNmEKOifH0UD68hd0YB2Wgwz3/u0snVpjidotOr+Eca8aSNSqZrXdInTSSopN0LB/syR I06g== Return-Path: Received: from ?IPv6:2a01:e0a:1dc:b1c0:c14:b5c9:8b27:499f? ([2a01:e0a:1dc:b1c0:c14:b5c9:8b27:499f]) by smtp.googlemail.com with ESMTPSA id l10sm19054483wrh.20.2019.09.30.13.21.40 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 30 Sep 2019 13:21:40 -0700 (PDT) Subject: Re: [PATCH] Fix algo constexpr tests in Debug mode To: Jonathan Wakely Cc: "libstdc++@gcc.gnu.org" , gcc-patches References: <368143e7-e151-9e37-f055-065044a57e7a@gmail.com> <20190927121109.GY9487@redhat.com> <20190927164507.GF9487@redhat.com> <20190930090346.GI9487@redhat.com> From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= Message-ID: Date: Mon, 30 Sep 2019 20:21:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190930090346.GI9487@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2019-09/txt/msg01808.txt.bz2 On 9/30/19 11:03 AM, Jonathan Wakely wrote: > On 28/09/19 23:12 +0200, François Dumont wrote: >> Here is what I just commited. > > Thanks. In my branch I had a lot more _GLIBCXX20_CONSTEXPR additions > in the debug mode headers. Is it not needed on __check_singular, > __foreign_iterator etc. ? Yes, I know, I just limited myself to fixing testsuite tests for the moment. I'll check if those need it too. > >> I try to use the asm trick in the _GLIBCXX_DEBUG_VERIFY_COND_AT but >> didn't notice any enhancement. So for now I kept my solution to just >> have a non-constexpr call compiler error. > > You won't see any improvement in the testsuite, because the compiler > flags for the testsuite suppress diagnostic notes. But I'd expect it > to give better output for users with the default compiler flags. Ok, I didn't know, I'll give it another try then outside testsuite. > > >> diff --git a/libstdc++-v3/include/bits/stl_algo.h >> b/libstdc++-v3/include/bits/stl_algo.h >> index a672f8b2b39..f25b8b76df6 100644 >> --- a/libstdc++-v3/include/bits/stl_algo.h >> +++ b/libstdc++-v3/include/bits/stl_algo.h >> @@ -5054,8 +5054,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO >>    *  @param  __last1   Another iterator. >>    *  @param  __last2   Another iterator. >>    *  @param  __result  An iterator pointing to the end of the merged >> range. >> -   *  @return         An iterator pointing to the first element >> not less >> -   *                  than @e val. >> +   *  @return   An output iterator equal to @p __result + (__last1 - >> __first1) >> +   *            + (__last2 - __first2). >>    * >>    *  Merges the ranges @p [__first1,__last1) and @p >> [__first2,__last2) into >>    *  the sorted range @p [__result, __result + (__last1-__first1) + >> @@ -5102,8 +5102,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO >>    *  @param  __last2   Another iterator. >>    *  @param  __result  An iterator pointing to the end of the merged >> range. >>    *  @param  __comp    A functor to use for comparisons. >> -   *  @return         An iterator pointing to the first element "not >> less >> -   *                  than" @e val. >> +   *  @return   An output iterator equal to @p __result + (__last1 - >> __first1) >> +   *            + (__last2 - __first2). >>    * >>    *  Merges the ranges @p [__first1,__last1) and @p >> [__first2,__last2) into >>    *  the sorted range @p [__result, __result + (__last1-__first1) + > > These changes are fine but should have been a separate, unrelated > commit. Ok, sorry, I consider that just a comment change was fine. > > >> @@ -157,10 +192,16 @@ namespace __gnu_debug >>    *  otherwise. >>   */ >>   template >> +    _GLIBCXX20_CONSTEXPR >>     inline bool >>     __valid_range(_InputIterator __first, _InputIterator __last, >>           typename _Distance_traits<_InputIterator>::__type& __dist) >>     { >> +#ifdef __cpp_lib_is_constant_evaluated >> +      if (std::is_constant_evaluated()) >> +    // Detected by the compiler directly. >> +    return true; >> +#endif > > Should this be using the built-in, not the C++20 function? > > > In practice it's probably equivalent, because the function is only > going to be constant-evaluated when called from C++20 code, and in > that case the std::is_constant_evaluated() function will be available. Yes, this is why I did it this way. And moreover it is using std::pair move assignment operator which is also C++20 constexpr. > > It just seems inconsistent to use the built-in in one place and not in > the other. It is also the reason why the simple simple __valid_range is not using the other anymore. Maybe once I'll have check all the algo calls I'll find out that this one need _GLIBCXX_CONSTEXPR. I got the sensation that library is being 'constexpr' decorated only when needed and when properly Standardise so are the Debug helpers. François