From patchwork Fri May 16 11:09:32 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 349573 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 5B860140082 for ; Fri, 16 May 2014 21:09:46 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=VDsci5Z52PU5fQTB+ T0MW5Sh6O+qVs2jr1b9yX8e6JWeGwwKP8aOKPkksi072aUYailbm7Y/m45xPxcj8 a915b4UYJ0U5CQSoGliF1w4nDqoNB/RAAhNMPyU+16pdif2Zat8BF7/GqIYbnx+m UInMwllUHJSV3QFwGLmNl9O5eM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=TAa5UoJzEnq4x8c13aVHNff R6Mk=; b=nnYPwIXpcCBmYW8Sn9Guf/xPcQloUw4YyYRmFGotw2orjphla0DuPgf DaAsfYtnLTqiLFofEvT97TNh0873ojiqfx3J6IGT38gzqS+oHaAfsnirkVUqTuO9 YLAGFh8tk4PWiqoLbpoqDZ/pVA6wsmN415Da6NL5oMy6wDN8L1zc= Received: (qmail 2161 invoked by alias); 16 May 2014 11:09:38 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 2138 invoked by uid 89); 16 May 2014 11:09:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham 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 ESMTP; Fri, 16 May 2014 11:09:36 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s4GB9YSB031508 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 16 May 2014 07:09:34 -0400 Received: from localhost (vpn1-5-128.ams2.redhat.com [10.36.5.128]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s4GB9WAT025844; Fri, 16 May 2014 07:09:33 -0400 Date: Fri, 16 May 2014 12:09:32 +0100 From: Jonathan Wakely To: Ed Smith-Rowland <3dw4rd@verizon.net> Cc: Daniel =?iso-8859-1?Q?Kr=FCgler?= , gcc-patches , "libstdc++@gcc.gnu.org" Subject: Re: [PATCH, libstdc++/61166] overflow when parse number in std::duration operator"" Message-ID: <20140516110931.GA29145@redhat.com> References: <537371EF.9080901@verizon.net> <53737CF4.9090706@verizon.net> <20140514153649.GZ10556@redhat.com> <20140515190317.GB26271@redhat.com> <53757DDF.6010909@verizon.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <53757DDF.6010909@verizon.net> User-Agent: Mutt/1.5.23 (2014-03-12) On 15/05/14 22:54 -0400, Ed Smith-Rowland wrote: >On 05/15/2014 03:03 PM, Jonathan Wakely wrote: >>Here's a finished patch to simplify >> >>Tested x86_64-linux. Ed, any objection to this version? >> >This looks great, thanks! I committed that to trunk, I'll put it on the 4.9 branch too. >Having done that should we actually stop using it as suggested in the >bug trail? ;-) I was going to do that, then realised that there's a defect in the standard where it requires overflow in duration integer literals to be diagnosed. That's only possible with literal operator templates, so I think we should keep your _Parse_int code, but apply the attached change to detect overflow. As the TODO comment says, it should be sufficient to simply instantiate integral_constant<_Rep, _Val::value> to give a diagnostic when _Rep{_Value::value} is narrowing, but GCC only gives a warning for it, and that's suppressed in a system header, so I do an explicit static_assert. That could be replaced with ... #pragma GCC diagnostic push #pragma GCC diagnostic error "-Woverflow" #pragma GCC diagnostic error "-Wsystem-headers" template constexpr _Dur __check_overflow() { using _Val = __parse_int::_Parse_int<_Digits...>; using _Rep = typename _Dur::rep; return _Dur{integral_constant<_Rep, _Val::value>::value}; } #pragma GCC diagnostic pop ... but I have plans to do that sort of thing more widely, which I'll deal with another time as part of https://gcc.gnu.org/PR50871 and/or https://gcc.gnu.org/PR58876 (what do other people think about using diagnostic pragmas to locally re-enable diagnostics in our headers?) Tested x86_64-linux, committed to trunk. commit 361c9b79e0b1c7f2435f5b0b369a139b216dee90 Author: Jonathan Wakely Date: Fri May 16 10:31:38 2014 +0100 * include/bits/parse_numbers.h (__parse_int::_Number_help): Check for overflow. * include/std/chrono (chrono_literals::__select_type::_Select_type): Remove. (chrono_literals::_Checked_integral_constant): Define. Simplify UDL operator templates and check for overflow. * testsuite/20_util/duration/literals/range.cc: New. diff --git a/libstdc++-v3/include/bits/parse_numbers.h b/libstdc++-v3/include/bits/parse_numbers.h index 0a42381a..a29d127 100644 --- a/libstdc++-v3/include/bits/parse_numbers.h +++ b/libstdc++-v3/include/bits/parse_numbers.h @@ -193,6 +193,7 @@ namespace __parse_int _Pow / (_Base * __valid_digit::value), _Digs...>; using type = __ull_constant<_Pow * __digit::value + __next::type::value>; + static_assert((type::value / _Pow) == __digit::value, "overflow"); }; template diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono index b114e02..39ad5e3 100644 --- a/libstdc++-v3/include/std/chrono +++ b/libstdc++-v3/include/std/chrono @@ -787,117 +787,79 @@ _GLIBCXX_END_NAMESPACE_VERSION inline namespace chrono_literals { - namespace __select_type - { - - using namespace __parse_int; - - template - struct _Select_type - : conditional< - _Val <= static_cast - (numeric_limits::max()), - _Dur, void> - { - static constexpr typename _Select_type::type - value{static_cast(_Val)}; - }; - - template - constexpr typename _Select_type<_Val, _Dur>::type - _Select_type<_Val, _Dur>::value; + template + struct _Checked_integral_constant + : integral_constant<_Rep, static_cast<_Rep>(_Val)> + { + static_assert(_Checked_integral_constant::value > 0 + && _Checked_integral_constant::value == _Val, + "literal value cannot be represented by duration type"); + }; - } // __select_type + template + constexpr _Dur __check_overflow() + { + using _Val = __parse_int::_Parse_int<_Digits...>; + using _Rep = typename _Dur::rep; + // TODO: should be simply integral_constant<_Rep, _Val::value> + // but GCC doesn't reject narrowing conversions to _Rep. + using _CheckedVal = _Checked_integral_constant<_Rep, _Val::value>; + return _Dur{_CheckedVal::value}; + } constexpr chrono::duration> operator""h(long double __hours) { return chrono::duration>{__hours}; } template - constexpr typename - __select_type::_Select_type<__select_int::_Select_int<_Digits...>::value, - chrono::hours>::type + constexpr chrono::hours operator""h() - { - return __select_type::_Select_type< - __select_int::_Select_int<_Digits...>::value, - chrono::hours>::value; - } + { return __check_overflow(); } constexpr chrono::duration> operator""min(long double __mins) { return chrono::duration>{__mins}; } template - constexpr typename - __select_type::_Select_type<__select_int::_Select_int<_Digits...>::value, - chrono::minutes>::type + constexpr chrono::minutes operator""min() - { - return __select_type::_Select_type< - __select_int::_Select_int<_Digits...>::value, - chrono::minutes>::value; - } + { return __check_overflow(); } constexpr chrono::duration operator""s(long double __secs) { return chrono::duration{__secs}; } template - constexpr typename - __select_type::_Select_type<__select_int::_Select_int<_Digits...>::value, - chrono::seconds>::type + constexpr chrono::seconds operator""s() - { - return __select_type::_Select_type< - __select_int::_Select_int<_Digits...>::value, - chrono::seconds>::value; - } + { return __check_overflow(); } constexpr chrono::duration operator""ms(long double __msecs) { return chrono::duration{__msecs}; } template - constexpr typename - __select_type::_Select_type<__select_int::_Select_int<_Digits...>::value, - chrono::milliseconds>::type + constexpr chrono::milliseconds operator""ms() - { - return __select_type::_Select_type< - __select_int::_Select_int<_Digits...>::value, - chrono::milliseconds>::value; - } + { return __check_overflow(); } constexpr chrono::duration operator""us(long double __usecs) { return chrono::duration{__usecs}; } template - constexpr typename - __select_type::_Select_type<__select_int::_Select_int<_Digits...>::value, - chrono::microseconds>::type + constexpr chrono::microseconds operator""us() - { - return __select_type::_Select_type< - __select_int::_Select_int<_Digits...>::value, - chrono::microseconds>::value; - } + { return __check_overflow(); } constexpr chrono::duration operator""ns(long double __nsecs) { return chrono::duration{__nsecs}; } template - constexpr typename - __select_type::_Select_type<__select_int::_Select_int<_Digits...>::value, - chrono::nanoseconds>::type + constexpr chrono::nanoseconds operator""ns() - { - return __select_type::_Select_type< - __select_int::_Select_int<_Digits...>::value, - chrono::nanoseconds>::value; - } + { return __check_overflow(); } } // inline namespace chrono_literals } // inline namespace literals diff --git a/libstdc++-v3/testsuite/20_util/duration/literals/range.cc b/libstdc++-v3/testsuite/20_util/duration/literals/range.cc new file mode 100644 index 0000000..c005df6 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/duration/literals/range.cc @@ -0,0 +1,31 @@ +// { dg-do compile } +// { dg-options "-std=gnu++1y" } + +// Copyright (C) 2014 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +#include + +void +test01() +{ + using namespace std::literals::chrono_literals; + + // std::numeric_limits::max() == 9223372036854775807; + auto h = 9223372036854775808h; + // { dg-error "cannot be represented" "" { target *-*-* } 794 } +}