From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 48597 invoked by alias); 19 Sep 2015 07:31:08 -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 48570 invoked by uid 89); 19 Sep 2015 07:31:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-wi0-f180.google.com Received: from mail-wi0-f180.google.com (HELO mail-wi0-f180.google.com) (209.85.212.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sat, 19 Sep 2015 07:31:05 +0000 Received: by wicfx3 with SMTP id fx3so54372693wic.0; Sat, 19 Sep 2015 00:31:03 -0700 (PDT) X-Received: by 10.194.20.161 with SMTP id o1mr12841028wje.32.1442647862940; Sat, 19 Sep 2015 00:31:02 -0700 (PDT) Received: from [192.168.0.23] (arf62-1-82-237-250-248.fbx.proxad.net. [82.237.250.248]) by smtp.googlemail.com with ESMTPSA id r9sm12616581wjz.35.2015.09.19.00.31.01 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 19 Sep 2015 00:31:02 -0700 (PDT) From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= Subject: Re: vector lightweight debug mode To: Jonathan Wakely References: <55F71189.8080006@gmail.com> <20150914195038.GQ2631@redhat.com> <55F9C4F6.6030706@gmail.com> <20150916202953.GE2631@redhat.com> Cc: "libstdc++@gcc.gnu.org" , gcc-patches Message-ID: <55FD0F35.4010106@gmail.com> Date: Sat, 19 Sep 2015 09:09:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20150916202953.GE2631@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-SW-Source: 2015-09/txt/msg01472.txt.bz2 On 16/09/2015 22:29, Jonathan Wakely wrote: > >>> >>>> 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? Not that much dependencies. We only need formatters.h in this mode which has the following common includes: #include #include and if rtti is enabled the less common: #include We would just leverage on the good job done to diagnose problems. > > >>>> @@ -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. > (Written before Christopher reply:) At least program will compile only if iterator is coming from a vector of the same type. So behavior is undefined only if user pass an invalid iterator which is exactly what this check tries to detect, isn't it paradoxical ? If this undefined behavior results in the program abortion this is what should happen anyway. If it doesn't abort then the program will definitely not behaves as expected so this check doesn't make anything worst, no ? François