Mailing-List: contact stdcxx-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: stdcxx-dev@incubator.apache.org
Received-SPF: pass (herse.apache.org: local policy)
Message-ID: <45CA7087.6050903@roguewave.com>
Date: Wed, 07 Feb 2007 17:36:23 -0700
From: Martin Sebor <sebor@roguewave.com>
Organization: Rogue Wave Software
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.8.1.2pre) Gecko/20070111 SeaMonkey/1.1
MIME-Version: 1.0
To: stdcxx-dev@incubator.apache.org
Subject: Re: [PATCH] Fix of STDCXX-268, STDCXX-331
References: <45C9F602.5050804@kyiv.vdiweb.com>
In-Reply-To: <45C9F602.5050804@kyiv.vdiweb.com>
Content-Type: text/plain; charset=KOI8-R; format=flowed
Content-Transfer-Encoding: 7bit

Farid Zaripov wrote:
>   Attached is a proposed patch for fix the bugs STDCXX-268 and STDCXX-331.

If the bodies of the functions are the same wouldn't be better
to call one from the other rather than introducing the macro?
(The macro makes it impossible to step through the functions
in most debuggers. I know we have macros in there already but
we don't need to make things worse by adding more :)

Or are we invoking the macro with arguments of different types
each time (Iterator, pointer, const_pointer) because we're
working around the lack of member templates? (We should
discuss and decide if we want to keep these workarounds or
if it's time to get rid of them and assume that the compiler
is reasonably modern).

Btw., I wonder if we could simplify (optimize) this code so
as to call erase(__start, __it) in the catch block instead
of looping (the idea is that the range form of erase() might
be more efficient than calling the single form repeatedly):

+#  define _RWSTD_LIST_SAFE_INSERT_RANGE(__it, __first, __last)  \
+    _RWSTD_ASSERT_RANGE (begin (), __it);                       \
+    _RWSTD_ASSERT_RANGE (__first, __last);                      \
+    size_type __n = 0;                                          \
+    _TRY {                                                      \
+        for ( ; !(__first == __last); ++__first, ++__n)         \
+            insert (__it, *__first);                            \
+    } _CATCH (...) {                                            \
+        for ( ; __n; --__n) {                                   \
+            iterator __prev = __it;                             \
+            erase (--__prev);                                   \
+        }                                                       \
+        _RETHROW;                                               \
+    } typedef void dummy_t
+
+

[Oh, also, be careful to privatize all names: dummy_t should
be __dummy_t (or __rw_dummy or something like that).]

Martin

> 
>   Actually this patch fixes the bug STDCXX-331, but since the std::list
> constructor uses insert() method so the bug STDCXX-268 is fixed 
> automatically.
> 
>   ChangeLog:
>   STDCXX-268
>   STDCXX-331
>   * list (_RWSTD_LIST_SAFE_INSERT_RANGE): New macro for exception
>   safe inserting the range into the list.
>   (_C_insert): Used _RWSTD_LIST_SAFE_INSERT_RANGE macro.
>   (_C_insert): Added try/catch with removing the inserted elements
>   if exception was thrown.
> 
> Farid.
>