From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13982 invoked by alias); 16 Sep 2015 20:29:57 -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 12068 invoked by uid 89); 16 Sep 2015 20:29:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 16 Sep 2015 20:29:56 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id E0E028C1DA; Wed, 16 Sep 2015 20:29:54 +0000 (UTC) Received: from localhost (ovpn-116-24.ams2.redhat.com [10.36.116.24]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8GKTr2O009611; Wed, 16 Sep 2015 16:29:54 -0400 Date: Wed, 16 Sep 2015 20:53:00 -0000 From: Jonathan Wakely To: =?iso-8859-1?Q?Fran=E7ois?= Dumont Cc: "libstdc++@gcc.gnu.org" , gcc-patches Subject: Re: vector lightweight debug mode Message-ID: <20150916202953.GE2631@redhat.com> References: <55F71189.8080006@gmail.com> <20150914195038.GQ2631@redhat.com> <55F9C4F6.6030706@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <55F9C4F6.6030706@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2015-09/txt/msg01240.txt.bz2 On 16/09/15 21:37 +0200, François Dumont wrote: >On 14/09/2015 21:50, Jonathan Wakely wrote: >> On 14/09/15 20:27 +0200, François Dumont wrote: >>> diff --git a/libstdc++-v3/include/bits/stl_vector.h >>> b/libstdc++-v3/include/bits/stl_vector.h >>> index 305d446..89a9aec 100644 >>> --- a/libstdc++-v3/include/bits/stl_vector.h >>> +++ b/libstdc++-v3/include/bits/stl_vector.h >>> @@ -449,6 +449,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER >>> vector& >>> operator=(vector&& __x) noexcept(_Alloc_traits::_S_nothrow_move()) >>> { >>> + __glibcxx_assert(this != &__x); >> >> Please don't do this, it fails in valid programs. The standard needs >> to be fixed in this regard. > >The debug mode check should be removed too then. Yes. >> >>> constexpr bool __move_storage = >>> _Alloc_traits::_S_propagate_on_move_assign() >>> || _Alloc_traits::_S_always_equal(); >>> @@ -778,7 +779,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER >>> */ >>> reference >>> operator[](size_type __n) _GLIBCXX_NOEXCEPT >>> - { return *(this->_M_impl._M_start + __n); } >>> + { >>> + __glibcxx_assert(__n < size()); >>> + return *(this->_M_impl._M_start + __n); >>> + } >> >> This could use __glibcxx_requires_subscript(__n), see the attached >> patch. > > I thought you didn't want to use anything from debug.h so I try to >do with only __glibcxx_assert coming from c++config. I think your patch >is missing include of debug.h. > > But I was going to propose to use _Error_formatter also in this >mode, I do not see any reason to not do so. The attached patch does just >that. That pulls in extra dependencies on I/O and fprintf and things, which can cause code size to increase. Is it really worth it? >>> @@ -1051,6 +1071,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER >>> iterator >>> insert(const_iterator __position, size_type __n, const >>> value_type& __x) >>> { >>> + __glibcxx_assert(__position >= cbegin() && __position <= cend()); >>> difference_type __offset = __position - cbegin(); >>> _M_fill_insert(begin() + __offset, __n, __x); >>> return begin() + __offset; >> >> This is undefined behaviour, so I'd rather not add this check (I know >> it's on the google branch, but it's still undefined behaviour). > >Why ? Because of the >= operator usage ? Is the attached patch better ? >< and == operators are well defined for a random access iterator, no ? No, because it is undefined to compare iterators that belong to different containers, or to compare pointers that point to different arrays.