From d055abd1958879ff1775772cfb0d5039e975fe9c Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Thu, 29 Jul 2021 18:40:54 +0200 Subject: [PATCH] Assert that special handling of '0' padding does what it should MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When formatting numbers, if the fill character used to left-pad to field widths is '0', the code delegates that padding to the QLocaleData's ZeroPadded formatting option. Since we want the zeros before any minus sign, and don't want to subsequently add more zeros before it, check that this has worked as expected when calling replaceArgEscapes(), to confirm that it doesn't need to worry about that. Add some tests that verify the expected behavior. In the process, tidy up the code doing this. Rename a local variable to match our coding style, split a long line. Pick-to: 6.2 Change-Id: I7cc430c5bceb006cf4e226bca33da16bd2bb1937 Reviewed-by: Ulf Hermann Reviewed-by: Mårten Nordheim --- src/corelib/text/qstring.cpp | 47 ++++++++---- .../auto/corelib/text/qstring/tst_qstring.cpp | 76 ++++++++++++++----- 2 files changed, 91 insertions(+), 32 deletions(-) diff --git a/src/corelib/text/qstring.cpp b/src/corelib/text/qstring.cpp index 6674d7a026..ddb12effd0 100644 --- a/src/corelib/text/qstring.cpp +++ b/src/corelib/text/qstring.cpp @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2020 The Qt Company Ltd. +** Copyright (C) 2021 The Qt Company Ltd. ** Copyright (C) 2020 Intel Corporation. ** Copyright (C) 2019 Mail.ru Group. ** Contact: https://www.qt.io/licensing/ @@ -8050,22 +8050,28 @@ QString QString::arg(qlonglong a, int fieldWidth, int base, QChar fillChar) cons } unsigned flags = QLocaleData::NoFlags; + // ZeroPadded sorts out left-padding when the fill is zero, to the right of sign: if (fillChar == QLatin1Char('0')) flags = QLocaleData::ZeroPadded; QString arg; - if (d.occurrences > d.locale_occurrences) + if (d.occurrences > d.locale_occurrences) { arg = QLocaleData::c()->longLongToString(a, -1, base, fieldWidth, flags); + Q_ASSERT(fillChar != QLatin1Char('0') || !qIsFinite(a) + || fieldWidth <= arg.length()); + } - QString locale_arg; + QString localeArg; if (d.locale_occurrences > 0) { QLocale locale; if (!(locale.numberOptions() & QLocale::OmitGroupSeparator)) flags |= QLocaleData::GroupDigits; - locale_arg = locale.d->m_data->longLongToString(a, -1, base, fieldWidth, flags); + localeArg = locale.d->m_data->longLongToString(a, -1, base, fieldWidth, flags); + Q_ASSERT(fillChar != QLatin1Char('0') || !qIsFinite(a) + || fieldWidth <= localeArg.length()); } - return replaceArgEscapes(*this, d, fieldWidth, arg, locale_arg, fillChar); + return replaceArgEscapes(*this, d, fieldWidth, arg, localeArg, fillChar); } /*! @@ -8092,22 +8098,28 @@ QString QString::arg(qulonglong a, int fieldWidth, int base, QChar fillChar) con } unsigned flags = QLocaleData::NoFlags; + // ZeroPadded sorts out left-padding when the fill is zero, to the right of sign: if (fillChar == QLatin1Char('0')) flags = QLocaleData::ZeroPadded; QString arg; - if (d.occurrences > d.locale_occurrences) + if (d.occurrences > d.locale_occurrences) { arg = QLocaleData::c()->unsLongLongToString(a, -1, base, fieldWidth, flags); + Q_ASSERT(fillChar != QLatin1Char('0') || !qIsFinite(a) + || fieldWidth <= arg.length()); + } - QString locale_arg; + QString localeArg; if (d.locale_occurrences > 0) { QLocale locale; if (!(locale.numberOptions() & QLocale::OmitGroupSeparator)) flags |= QLocaleData::GroupDigits; - locale_arg = locale.d->m_data->unsLongLongToString(a, -1, base, fieldWidth, flags); + localeArg = locale.d->m_data->unsLongLongToString(a, -1, base, fieldWidth, flags); + Q_ASSERT(fillChar != QLatin1Char('0') || !qIsFinite(a) + || fieldWidth <= localeArg.length()); } - return replaceArgEscapes(*this, d, fieldWidth, arg, locale_arg, fillChar); + return replaceArgEscapes(*this, d, fieldWidth, arg, localeArg, fillChar); } /*! @@ -8186,6 +8198,7 @@ QString QString::arg(double a, int fieldWidth, char format, int precision, QChar } unsigned flags = QLocaleData::NoFlags; + // ZeroPadded sorts out left-padding when the fill is zero, to the right of sign: if (fillChar == QLatin1Char('0')) flags |= QLocaleData::ZeroPadded; @@ -8211,10 +8224,14 @@ QString QString::arg(double a, int fieldWidth, char format, int precision, QChar } QString arg; - if (d.occurrences > d.locale_occurrences) - arg = QLocaleData::c()->doubleToString(a, precision, form, fieldWidth, flags | QLocaleData::ZeroPadExponent); + if (d.occurrences > d.locale_occurrences) { + arg = QLocaleData::c()->doubleToString(a, precision, form, fieldWidth, + flags | QLocaleData::ZeroPadExponent); + Q_ASSERT(fillChar != QLatin1Char('0') || !qIsFinite(a) + || fieldWidth <= arg.length()); + } - QString locale_arg; + QString localeArg; if (d.locale_occurrences > 0) { QLocale locale; @@ -8225,10 +8242,12 @@ QString QString::arg(double a, int fieldWidth, char format, int precision, QChar flags |= QLocaleData::ZeroPadExponent; if (numberOptions & QLocale::IncludeTrailingZeroesAfterDot) flags |= QLocaleData::AddTrailingZeroes; - locale_arg = locale.d->m_data->doubleToString(a, precision, form, fieldWidth, flags); + localeArg = locale.d->m_data->doubleToString(a, precision, form, fieldWidth, flags); + Q_ASSERT(fillChar != QLatin1Char('0') || !qIsFinite(a) + || fieldWidth <= localeArg.length()); } - return replaceArgEscapes(*this, d, fieldWidth, arg, locale_arg, fillChar); + return replaceArgEscapes(*this, d, fieldWidth, arg, localeArg, fillChar); } static inline char16_t to_unicode(const QChar c) { return c.unicode(); } diff --git a/tests/auto/corelib/text/qstring/tst_qstring.cpp b/tests/auto/corelib/text/qstring/tst_qstring.cpp index fd64d72121..733be3c353 100644 --- a/tests/auto/corelib/text/qstring/tst_qstring.cpp +++ b/tests/auto/corelib/text/qstring/tst_qstring.cpp @@ -6540,6 +6540,34 @@ void tst_QString::nanAndInf() valueFromString = valueAsString.toFloat(); QVERIFY(value == -qInf()); QVERIFY(qIsInf(valueFromString)); + + // Check that .arg(inf-or-nan, wide, fmt, 3, '0') padds with zeros + QString form = QStringLiteral("%1"); + QCOMPARE(form.arg(qInf(), 5, 'f', 3, '0'), u"00inf"); + QCOMPARE(form.arg(qInf(), -5, 'f', 3, '0'), u"inf00"); + QCOMPARE(form.arg(-qInf(), 6, 'f', 3, '0'), u"00-inf"); + QCOMPARE(form.arg(-qInf(), -6, 'f', 3, '0'), u"-inf00"); + QCOMPARE(form.arg(qQNaN(), -5, 'f', 3, '0'), u"nan00"); + QCOMPARE(form.arg(qInf(), 5, 'e', 3, '0'), u"00inf"); + QCOMPARE(form.arg(qInf(), -5, 'e', 3, '0'), u"inf00"); + QCOMPARE(form.arg(-qInf(), 6, 'e', 3, '0'), u"00-inf"); + QCOMPARE(form.arg(-qInf(), -6, 'e', 3, '0'), u"-inf00"); + QCOMPARE(form.arg(qQNaN(), -5, 'e', 3, '0'), u"nan00"); + QCOMPARE(form.arg(qInf(), 5, 'E', 3, '0'), u"00INF"); + QCOMPARE(form.arg(qInf(), -5, 'E', 3, '0'), u"INF00"); + QCOMPARE(form.arg(-qInf(), 6, 'E', 3, '0'), u"00-INF"); + QCOMPARE(form.arg(-qInf(), -6, 'E', 3, '0'), u"-INF00"); + QCOMPARE(form.arg(qQNaN(), -5, 'E', 3, '0'), u"NAN00"); + QCOMPARE(form.arg(qInf(), 5, 'g', 3, '0'), u"00inf"); + QCOMPARE(form.arg(qInf(), -5, 'g', 3, '0'), u"inf00"); + QCOMPARE(form.arg(-qInf(), 6, 'g', 3, '0'), u"00-inf"); + QCOMPARE(form.arg(-qInf(), -6, 'g', 3, '0'), u"-inf00"); + QCOMPARE(form.arg(qQNaN(), -5, 'g', 3, '0'), u"nan00"); + QCOMPARE(form.arg(qInf(), 5, 'G', 3, '0'), u"00INF"); + QCOMPARE(form.arg(qInf(), -5, 'G', 3, '0'), u"INF00"); + QCOMPARE(form.arg(-qInf(), 6, 'G', 3, '0'), u"00-INF"); + QCOMPARE(form.arg(-qInf(), -6, 'G', 3, '0'), u"-INF00"); + QCOMPARE(form.arg(qQNaN(), -5, 'G', 3, '0'), u"NAN00"); } void tst_QString::arg_fillChar_data() @@ -6550,27 +6578,39 @@ void tst_QString::arg_fillChar_data() QTest::addColumn("fillChars"); QTest::addColumn("expected"); - QList replaceValues; - IntList widths; - QString fillChars; + using DataList = QList; - replaceValues << QVariant((int)5) << QVariant(QString("f")) << QVariant((int)0); - widths << 3 << 2 << 5; - QTest::newRow("str0") << QString("%1%2%3") << replaceValues << widths << QString("abc") << QString("aa5bfcccc0"); + QTest::newRow("str0") + << QStringLiteral("%1%2%3") + << DataList{QVariant(int(5)), QVariant(QString("f")), QVariant(int(0))} + << IntList{3, 2, 5} << QString("abc") << QString("aa5bfcccc0"); - replaceValues.clear(); - widths.clear(); - replaceValues << QVariant((int)5.5) << QVariant(QString("foo")) << QVariant((qulonglong)INT_MAX); - widths << 10 << 2 << 5; - QTest::newRow("str1") << QString("%3.%1.%3.%2") << replaceValues << widths << QString("0 c") - << QString("2147483647.0000000005.2147483647.foo"); + QTest::newRow("str1") + << QStringLiteral("%3.%1.%3.%2") + << DataList{QVariant(int(5)), QVariant(QString("foo")), QVariant(qulonglong(INT_MAX))} + << IntList{10, 2, 5} << QString("0 c") << QString("2147483647.0000000005.2147483647.foo"); - replaceValues.clear(); - widths.clear(); - replaceValues << QVariant(QString("fisk")); - widths << 100; - QTest::newRow("str2") << QString("%9 og poteter") << replaceValues << widths << QString("f") - << QString("fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffisk og poteter"); + QTest::newRow("str2") + << QStringLiteral("%9 og poteter") + << DataList{QVariant(QString("fisk"))} << IntList{100} << QString("f") + << QString("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + "fffffffffffffffffffffffffffffffffffffisk og poteter"); + + // Left-padding with zeros fits them before the minus sign: + QTest::newRow("zero-left") + << QStringLiteral("%1 %2 %3 %4 %5 %6") + << DataList{QVariant(-314.0), QVariant(-3.14e8), QVariant(-3.14e12), + QVariant(-.0314), QVariant(-3.14e-7), QVariant(-3.14e-13)} + << IntList{6, 11, 11, 9, 11, 11} << QStringLiteral("000000") + << QStringLiteral("-00314 -003.14e+08 -003.14e+12 -000.0314 -003.14e-07 -003.14e-13"); + + // Right-padding with zeros is not so sensible: + QTest::newRow("zero-right") + << QStringLiteral("%1 %2 %3 %4 %5 %6") + << DataList{QVariant(-314.0), QVariant(-3.14e8), QVariant(-3.14e12), + QVariant(-.0314), QVariant(-3.14e-7), QVariant(-3.14e-13)} + << IntList{-6, -11, -11, -9, -11, -11} << QStringLiteral("000000") + << QStringLiteral("-31400 -3.14e+0800 -3.14e+1200 -0.031400 -3.14e-0700 -3.14e-1300"); } void tst_QString::arg_fillChar()