From patchwork Thu Aug 1 21:32:26 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 1968049 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=UeoDyHBl; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WZj7771nxz1yZl for ; Fri, 2 Aug 2024 07:40:18 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E340C385DDE4 for ; Thu, 1 Aug 2024 21:40:16 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 2F9F33858D20 for ; Thu, 1 Aug 2024 21:39:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2F9F33858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2F9F33858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1722548378; cv=none; b=h+KryOHBy8/XACYS35ZLoI/+C/HtrF+qHmZlKMdPvO0BTXOkfHwlMONqddytSQ1wqtxOnXW8B4GpzXUmzE6vSQhD6NeHFya6f8MPf1pIrPYpGXjIn58UFD/fXF//2jNs3lvWIjF/eXc+m868vJ7zr3c3MCp9rm9FvMbCFAT2eM8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1722548378; c=relaxed/simple; bh=7pC7MAt3yfIwm5iIAS1GIPeDHYRlQrLC8p57HligjHs=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=REEg/Mcrkd8JweWyJdauXBijpMy7Uinj19rLAGqT3+ijrbEhN18zQbyIK7b5398xh7cVtq2fb/FZIU6XU5sPgQLb2Wcc+f1r2ZPxBQVuYC+DVMWP3LUgZMXVa0x3hF4yR/StqvX7eqeFlfWE8+ERS6N5GpjFg6QG6IBg8/oJ0SQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1722548375; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=S6MQO8aXAedQy0d4kWsYVfG2QGoyEpD5Wz89qW+FjBo=; b=UeoDyHBlpCmBRqUQAzsjxkgAo7gDEpAHiF2vbpGvheVmE3SiWfIpy0XmKeGfNj0+JAzgvg QaMiwFnuB/9PFmLCEW6Y0KlNIzQ5l2noXIfpCTVlo0Ym4Uk1NRJN9HCM4geY8QkEwVWEqm jLAk6kmTklSM8HiV2Jehfe4/fIQ6oZE= Received: from mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-201-azW49PWHMJW3iwpeedleoA-1; Thu, 01 Aug 2024 17:39:31 -0400 X-MC-Unique: azW49PWHMJW3iwpeedleoA-1 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id F31001955D4C; Thu, 1 Aug 2024 21:39:29 +0000 (UTC) Received: from localhost (unknown [10.42.28.21]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 397E219560AE; Thu, 1 Aug 2024 21:39:28 +0000 (UTC) From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [PATCH] libstdc++: Only use std::time_put in std::format for non-C locales Date: Thu, 1 Aug 2024 22:32:26 +0100 Message-ID: <20240801213927.388966-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces~incoming=patchwork.ozlabs.org@gcc.gnu.org Tested x86_64-linux and sparc-solaris. I really need to add some tests that use modified formats like %EY and %OS for locales that actually have special forms for those. We don't have any such tests now, and with this change most of the _M_locale_fmt code paths are no longer exercised at all. So better tests needed. -- >8 -- When testing on Solaris I noticed that std/time/year/io.cc was FAILing because the year 1642 was being formatted as "+(" by %Ey. This turns out to be because we defer to std::time_put for modified conversion specs, and std::time_put uses std::strftime, and that's undefined for years before 1970. In particular, years before 1900 mean that the tm_year field is negative, which then causes incorrect results from strftime on at least Solaris and AIX. I've raised the general problem with LWG, but we can fix the FAILing test case (and probably improve performance slightly) by ignoring the E and O modifiers when the formatting locale is the "C" locale. The modifiers have no effect for the C locale, so we can just treat %Ey as %y and format it directly. This doesn't fix anything when the formatting locale isn't the C locale, but that case is not adequately tested, so doesn't cause any FAIL right now! The naïve fix would be simply: if (__mod) if (auto __loc = _M_locale(__ctx); __loc != locale::classic()) // ... However when the format string doesn't use the 'L' option, _M_locale always returns locale::classic(). In that case, we make a copy of the classic locale (which calls the non-inline copy constructor in the library), then make another copy of the classic locale, then compare the two. We can avoid all that by checking for the 'L' option first, instead of letting _M_locale do that: if (__mod && _M_spec._M_localized) if (auto __loc = __ctx.locale(); __loc != locale::classic()) // ... We could optimize this further if we had a __is_classic(__loc) function that would do the __loc == locale::classic() check without making any copies or non-inline calls. That would require examining the locale's _M_impl member, and probably require checking its name, because the locale::_S_classic singleton is not exported from the library. For _M_S the change is slightly different from the other functions, because if we skip using std::time_put for %OS then we fall through to the code that potentially prints fractional seconds, but the %OS format only prints whole seconds. So we need to format whole seconds directly when not using std::time_put, instead of falling through to the code below. libstdc++-v3/ChangeLog: * include/bits/chrono_io.h (__formatter_chrono::_M_C_y_Y): Ignore modifiers unless the formatting locale is not the C locale. (__formatter_chrono::_M_d_e): Likewise. (__formatter_chrono::_M_H_I): Likewise. (__formatter_chrono::_M_m): Likewise. (__formatter_chrono::_M_M): Likewise. (__formatter_chrono::_M_S): Likewise. (__formatter_chrono::_M_u_w): Likewise. (__formatter_chrono::_M_U_V_W): Likewise. --- libstdc++-v3/include/bits/chrono_io.h | 133 ++++++++++++++------------ 1 file changed, 74 insertions(+), 59 deletions(-) diff --git a/libstdc++-v3/include/bits/chrono_io.h b/libstdc++-v3/include/bits/chrono_io.h index a449ffdc558..38a0b002c81 100644 --- a/libstdc++-v3/include/bits/chrono_io.h +++ b/libstdc++-v3/include/bits/chrono_io.h @@ -917,13 +917,14 @@ namespace __format chrono::year __y = _S_year(__t); - if (__mod) [[unlikely]] - { - struct tm __tm{}; - __tm.tm_year = (int)__y - 1900; - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - __conv, __mod); - } + if (__mod && _M_spec._M_localized) [[unlikely]] + if (auto __loc = __ctx.locale(); __loc != locale::classic()) + { + struct tm __tm{}; + __tm.tm_year = (int)__y - 1900; + return _M_locale_fmt(std::move(__out), __loc, __tm, + __conv, __mod); + } basic_string<_CharT> __s; int __yi = (int)__y; @@ -985,13 +986,14 @@ namespace __format chrono::day __d = _S_day(__t); unsigned __i = (unsigned)__d; - if (__mod) [[unlikely]] - { - struct tm __tm{}; - __tm.tm_mday = __i; - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - (char)__conv, 'O'); - } + if (__mod && _M_spec._M_localized) [[unlikely]] + if (auto __loc = __ctx.locale(); __loc != locale::classic()) + { + struct tm __tm{}; + __tm.tm_mday = __i; + return _M_locale_fmt(std::move(__out), __loc, __tm, + (char)__conv, 'O'); + } auto __sv = _S_two_digits(__i); _CharT __buf[2]; @@ -1051,13 +1053,14 @@ namespace __format const auto __hms = _S_hms(__t); int __i = __hms.hours().count(); - if (__mod) [[unlikely]] - { - struct tm __tm{}; - __tm.tm_hour = __i; - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - (char)__conv, 'O'); - } + if (__mod && _M_spec._M_localized) [[unlikely]] + if (auto __loc = __ctx.locale(); __loc != locale::classic()) + { + struct tm __tm{}; + __tm.tm_hour = __i; + return _M_locale_fmt(std::move(__out), __loc, __tm, + (char)__conv, 'O'); + } if (__conv == _CharT('I')) { @@ -1109,13 +1112,14 @@ namespace __format auto __m = _S_month(__t); auto __i = (unsigned)__m; - if (__mod) [[unlikely]] // %Om - { - struct tm __tm{}; - __tm.tm_mon = __i - 1; - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - 'm', 'O'); - } + if (__mod && _M_spec._M_localized) [[unlikely]] // %Om + if (auto __loc = __ctx.locale(); __loc != locale::classic()) + { + struct tm __tm{}; + __tm.tm_mon = __i - 1; + return _M_locale_fmt(std::move(__out), __loc, __tm, + 'm', 'O'); + } return __format::__write(std::move(__out), _S_two_digits(__i)); } @@ -1131,13 +1135,14 @@ namespace __format auto __m = _S_hms(__t).minutes(); auto __i = __m.count(); - if (__mod) [[unlikely]] // %OM - { - struct tm __tm{}; - __tm.tm_min = __i; - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - 'M', 'O'); - } + if (__mod && _M_spec._M_localized) [[unlikely]] // %OM + if (auto __loc = __ctx.locale(); __loc != locale::classic()) + { + struct tm __tm{}; + __tm.tm_min = __i; + return _M_locale_fmt(std::move(__out), __loc, __tm, + 'M', 'O'); + } return __format::__write(std::move(__out), _S_two_digits(__i)); } @@ -1226,22 +1231,30 @@ namespace __format // %S Seconds as a decimal number. // %OS The locale's alternative representation. auto __hms = _S_hms(__t); + auto __s = __hms.seconds(); if (__mod) [[unlikely]] // %OS { - struct tm __tm{}; - __tm.tm_sec = (int)__hms.seconds().count(); - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - 'S', 'O'); + if (_M_spec._M_localized) + if (auto __loc = __ctx.locale(); __loc != locale::classic()) + { + struct tm __tm{}; + __tm.tm_sec = (int)__s.count(); + return _M_locale_fmt(std::move(__out), __loc, __tm, + 'S', 'O'); + } + + // %OS formats don't include subseconds, so just format that: + return __format::__write(std::move(__out), + _S_two_digits(__s.count())); } if constexpr (__hms.fractional_width == 0) __out = __format::__write(std::move(__out), - _S_two_digits(__hms.seconds().count())); + _S_two_digits(__s.count())); else { locale __loc = _M_locale(__ctx); - auto __s = __hms.seconds(); auto __ss = __hms.subseconds(); using rep = typename decltype(__ss)::rep; if constexpr (is_floating_point_v) @@ -1291,13 +1304,14 @@ namespace __format chrono::weekday __wd = _S_weekday(__t); - if (__mod) [[unlikely]] - { - struct tm __tm{}; - __tm.tm_wday = __wd.c_encoding(); - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - (char)__conv, 'O'); - } + if (__mod && _M_spec._M_localized) [[unlikely]] + if (auto __loc = __ctx.locale(); __loc != locale::classic()) + { + struct tm __tm{}; + __tm.tm_wday = __wd.c_encoding(); + return _M_locale_fmt(std::move(__out), __loc, __tm, + (char)__conv, 'O'); + } unsigned __wdi = __conv == 'u' ? __wd.iso_encoding() : __wd.c_encoding(); @@ -1320,17 +1334,18 @@ namespace __format auto __d = _S_days(__t); using _TDays = decltype(__d); // Either sys_days or local_days. - if (__mod) [[unlikely]] - { - const year_month_day __ymd(__d); - const year __y = __ymd.year(); - struct tm __tm{}; - __tm.tm_year = (int)__y - 1900; - __tm.tm_yday = (__d - _TDays(__y/January/1)).count(); - __tm.tm_wday = weekday(__d).c_encoding(); - return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm, - (char)__conv, 'O'); - } + if (__mod && _M_spec._M_localized) [[unlikely]] + if (auto __loc = __ctx.locale(); __loc != locale::classic()) + { + const year_month_day __ymd(__d); + const year __y = __ymd.year(); + struct tm __tm{}; + __tm.tm_year = (int)__y - 1900; + __tm.tm_yday = (__d - _TDays(__y/January/1)).count(); + __tm.tm_wday = weekday(__d).c_encoding(); + return _M_locale_fmt(std::move(__out), __loc, __tm, + (char)__conv, 'O'); + } _TDays __first; // First day of week 1. if (__conv == 'V') // W01 begins on Monday before first Thursday.