From stdcxx-dev-return-4303-apmail-incubator-stdcxx-dev-archive=incubator.apache.org@incubator.apache.org Mon Aug 13 03:43:27 2007 Return-Path: Delivered-To: apmail-incubator-stdcxx-dev-archive@www.apache.org Received: (qmail 51122 invoked from network); 13 Aug 2007 03:43:27 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 13 Aug 2007 03:43:27 -0000 Received: (qmail 52786 invoked by uid 500); 13 Aug 2007 03:43:25 -0000 Delivered-To: apmail-incubator-stdcxx-dev-archive@incubator.apache.org Received: (qmail 52771 invoked by uid 500); 13 Aug 2007 03:43:25 -0000 Mailing-List: contact stdcxx-dev-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: stdcxx-dev@incubator.apache.org Delivered-To: mailing list stdcxx-dev@incubator.apache.org Received: (qmail 52760 invoked by uid 99); 13 Aug 2007 03:43:25 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 12 Aug 2007 20:43:25 -0700 X-ASF-Spam-Status: No, hits=-0.0 required=10.0 tests=SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of msebor@gmail.com designates 209.85.146.179 as permitted sender) Received: from [209.85.146.179] (HELO wa-out-1112.google.com) (209.85.146.179) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 13 Aug 2007 03:43:19 +0000 Received: by wa-out-1112.google.com with SMTP id n4so4395969wag for ; Sun, 12 Aug 2007 20:42:59 -0700 (PDT) DKIM-Signature: a=rsa-sha1; c=relaxed/relaxed; d=gmail.com; s=beta; h=domainkey-signature:received:received:message-id:date:from:organization:user-agent:mime-version:to:subject:references:in-reply-to:content-type:content-transfer-encoding:sender; b=S7xUp0a8g4ODWHh+A3EftaXxShSPfw8EUaI9B8meae+fEca3g4h+JhRzTv3U81J4ZGpEz8p2/+s4qn+pQxQD4JHi9GMH6xhL3RYXkjGtT7nQSEhA8zj20HLLuRUqdkSyCXymUSLc3EupVIDhWRpynQXzYBc2oKLtmhLQNQ3gaMc= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:organization:user-agent:mime-version:to:subject:references:in-reply-to:content-type:content-transfer-encoding:sender; b=rGnFlvHRmsidmmkhZsJEN2vWqXOXvqTL0OFYkqKKErNpaSxMnn/09mkTf1icMankuHvXlWAWb4nTcdeMlAVl3LBkRdmr4diLaFAaBh4APisvzSE5qTJsEuR9r+ROkMIjOs59pYiQwh4U5x3kyq0KJe3DQKoHyOrLArmdU2AjD2s= Received: by 10.114.95.1 with SMTP id s1mr491301wab.1186976578919; Sun, 12 Aug 2007 20:42:58 -0700 (PDT) Received: from ?192.168.1.104? ( [71.229.200.170]) by mx.google.com with ESMTPS id m27sm9456321pof.2007.08.12.20.42.57 (version=TLSv1/SSLv3 cipher=RC4-MD5); Sun, 12 Aug 2007 20:42:57 -0700 (PDT) Message-ID: <46BFD340.5020701@roguewave.com> Date: Sun, 12 Aug 2007 21:42:56 -0600 From: Martin Sebor Organization: Rogue Wave Software User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.0.12) Gecko/20070719 Fedora/1.0.9-2.fc6 pango-text SeaMonkey/1.0.9 MIME-Version: 1.0 To: stdcxx-dev@incubator.apache.org Subject: Re: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results [take 2] References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: Martin Sebor X-Virus-Checked: Checked by ClamAV on apache.org Travis Vitek wrote: > Attached is a patch to enhance the num_put facet mt test. Threads > verify that the values they put compare equal to those put in the > primary thread. There are few outstanding issues here that we need to resolve before committing this patch... > [...] > @@ -58,150 +64,129 @@ [...] > > -#ifndef _RWSTD_NO_WCHAR_T > + // the value that we will be formatting > + int value_; I suggest to make the type of the variable double to make it possible to exercise the formatting of the fractional part of the value (for floating point types). > > - struct WideBuf: std::wstreambuf { > - int_type overflow (int_type c) { return c; } > - } wb; > + // holds the narrow/wide character representation of value_ and > + // the number of used 'charT' in each buffer. > + struct result { > + char ncs_ [BufferSize]; > + std::char_traits::off_type nlen_; This member doesn't appear to be used in the test. > > -#endif // _RWSTD_NO_WCHAR_T > + wchar_t wcs_ [BufferSize]; > + std::char_traits::off_type wlen_; Also unused. > + }; > > - struct Ios: std::ios { > - Ios () { this->init (0); } > - } io; > + result bool_; > + result long_; > + result ulong_; > + result llong_; > + result ullong_; > + result double_; > + result ldouble_; > + result pointer_; FYI: the naming convention we've been trying to follow for names referring to fundamental types is the one used by the C numeric limits macros: i.e., "dbl" for double and "ldbl" for long double. For pointers, we've been using "ptr." It comes in handy when using the preprocessor when parametrizing functions on these types (and when interacting with the test driver and command line options). > [...] > +#define countof(x) (sizeof (x) / sizeof (*x)) Since you seem to like it so much ;-) we might as well move this macro to some central test suite header (and rename it according to the naming convention). > [...] > + for (int i = 0; i != rw_opt_nloops; ++i) { > > - io.width (i % 16); > + // fill in the value and results for this locale > + const MyNumData& data = my_num_data [i % nlocales]; > > - // exercise postive and negative values > - const int ival = i & 1 ? -i : i; > - > + // construct a named locale and imbue it in the ios object > + // so that the locale is used not only by the num_put facet > + const std::locale loc = > + rw_opt_shared_locale ? data.locale_ > + : std::locale (data.locale_name_); > if (test_char) { > // exercise the narrow char specialization of the facet > > const std::num_put &np = > std::use_facet >(loc); > > - const std::ostreambuf_iterator iter (&sb); > + nio.imbue (loc); Calling imbue() on the ios object before calling rdbuf() on it prevents the same locale from being imbued in the streambuf. Did you intend for that to happen? > [...] > +#define TEST(sb, buf, cmp, io, p, fill, valueT, val, charT) \ > + sb.pubsetp (buf, countof (buf)); \ > + io.rdbuf (&sb); \ Is the call to rdbuf() necessary for every test or can it be done just once per thread? (Preferably before calling imbue() on the ios object.) > + *p.put (std::ostreambuf_iterator(&sb), \ > + io, fill, (valueT)(val)) = charT (); \ > + RW_ASSERT (!io.fail ()); \ > + RW_ASSERT (!rw_strncmp (buf, cmp)); > > - case put_long: > - np.put (iter, io, ' ', long (ival)); > - break; > +#define TEST_N(o, t, v) \ > + TEST(nsb, ncs, o.ncs_, nio, np, ' ', t, v, char) > > - case put_ulong: > - np.put (iter, io, ' ', (unsigned long)ival); > - break; > - > + TEST_N (data.bool_, bool, data.value_ != 0); > + TEST_N (data.long_, long, data.value_); > + TEST_N (data.ulong_, unsigned long, data.value_); I note you've changed the test from invoking one num_put member per iteration to invoking all members each iteration. I'm curious about your rationale for the change? (I don't necessarily disagree with the approach, just wondering what if anything led you to make the change.) FWIW, the one advantage to sticking with the original testing strategy is that it would let us eliminate the call to pubsetp() and (possibly) do away with the macros. [...] > @@ -266,23 +229,126 @@ > > /**************************************************************************/ > > + > static int > run_test (int, char**) > { [...] > + const char* const possible_locale_options[] = { > + locale_list, "C\0", 0 > + }; Same applies as in this post: http://www.mail-archive.com/stdcxx-dev@incubator.apache.org/msg04274.html > [...] > +#define SETUP(sb, buf, io, p, fill, valueT, val, charT, loc) \ > + sb.pubsetp (buf, countof (buf)); \ > + io.rdbuf (&sb); \ > + *p.put (std::ostreambuf_iterator(&sb), \ > + io, fill, (valueT)(val)) = charT (); \ > + rw_assert (!io.fail (), __FILE__, __LINE__, \ I don't think rw_assert() is appropriate here. If the facet fails to format the "master" value into the buffer here the rest of the test is most likely hosed anyway and shouldn't even be attempted. Wouldn't rw_fatal() be a better choice? Also, if you'd like to get rid of the macro (I would :), it should be possible to move the call to rdbuf() outside the loop and verify that the formatting was successful only once per iteration. [...] > + // avoid divide by zero in thread if there are no locales to test > + rw_fatal (nlocales != 0, 0, __LINE__, > + "failed to create one or more usable locales!"); Assuming we change rw_locales() to always return at least "C" this should never happen, right? (I.e., we'll be able to eliminate the call to rw_fatal() after the change.) Martin