1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Straighten out QLocaleData::numberToCLocale()

Browse Source 
It was a bit convoluted, making it hard to reason about parts. The end
of the whole number part can happen at the very end, after the loop,
or when we hit the exponent or fractional part. That means the check
of grouping rules at the end of the whole number part had to happen
after the loop as well as within it. To avoid duplication within the
loop, the handlingof exponent and fractional part was done within the
handling of grouping, reprising the exponent and fractional part after
they're handled earlier in the loop.

Package the decision-making for that handling in a lambda (and
document some relevant details there) so that the duplication is
harmless and move into the handling of the exponent and fractional
part separators, where it belongs. This lets the bulk of the loop body
simplify into a straightforward if/else-if cascade, making it easier
to reason about (in preparation for some imminent fixing of quirks and
corner cases).

Since reviewers needed to understand what the GroupSizes members mean,
in order to make sense of this, I've also added some comments to the
struct's declaration to at least let future readers know what's what.

This refactoring should be harmless and change nothing; it is also
needed by an imminent fix, that I shall pick back into LTS branches.

Pick-to: 6.9 6.8 6.5
Task-number: QTBUG-134913
Change-Id: Ib352aa9ab8341914e40d9c087b497db15a180ebb
Reviewed-by: Mate Barany <mate.barany@qt.io>
This commit is contained in:
Edward Welbourne 2025-03-21 17:54:11 +01:00
parent 0dab50a124
commit cdcaaaeee3
2 changed files with 68 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

115
src/corelib/text/qlocale.cpp
View File

@ -4621,6 +4621,20 @@ bool QLocaleData::numberToCLocale(QStringView s, QLocale::NumberOptions number_o
qsizetype last_separator_idx = -1;
qsizetype start_of_digits_idx = -1;
const QLocaleData::GroupSizes grouping = groupSizes();
const auto badLeastGroup = [&]() {
// In principle we could object to a complete absence of grouping, when
// digitsInGroup >= qMax(grouping.first, grouping.least), unless the
// locale itself would omit them. However, when merely not rejecting
// grouping separators, we have historically accepted ungrouped digits,
// so objecting now would break existing code.
if (last_separator_idx != -1) {
Q_ASSERT(!number_options.testFlag(QLocale::RejectGroupSeparator));
// Were there enough digits since the last group separator?
if (digitsInGroup != grouping.least)
return true;
}
return false;
};
// Floating-point details (non-integer modes):
qsizetype decpt_idx = -1;
@ -4634,75 +4648,74 @@ bool QLocaleData::numberToCLocale(QStringView s, QLocale::NumberOptions number_o
return false;
Q_ASSERT(tokens.index() > idx); // it always *should* advance (except on zero return)
// Note that out can only be '.', 'e' or an inf/NaN character if the
// mode allows it (else nextToken() would return 0 instead), so we don't
// need to check mode.
if (out == '.') {
// Fail if more than one decimal point or point after e
if (decpt_idx != -1 || exponent_idx != -1)
return false;
decpt_idx = idx;
// That's the end of the integral part - check size of last group:
if (badLeastGroup())
return false;
last_separator_idx = -1; // Process no separators after this
} else if (out == 'e') {
exponent_idx = idx;
}
if (number_options.testFlag(QLocale::RejectLeadingZeroInExponent)
&& exponent_idx != -1 && out == '0') {
// After the exponent there can only be '+', '-' or digits.
// If we find a '0' directly after some non-digit, then that is a
// leading zero, acceptable only if it is the whole exponent.
if (!tokens.done() && !isAsciiDigit(last))
return false;
}
if (number_options.testFlag(QLocale::RejectTrailingZeroesAfterDot) && decpt_idx >= 0) {
// In a fractional part, a 0 just before the exponent is trailing:
if (idx == exponent_idx && last == '0')
return false;
}
if (!number_options.testFlag(QLocale::RejectGroupSeparator)) {
if (isAsciiDigit(out)) {
if (start_of_digits_idx == -1)
start_of_digits_idx = idx;
++digitsInGroup;
} else if (out == ',') {
// Don't allow group chars after the decimal point or exponent
if (decpt_idx != -1 || exponent_idx != -1)
if (decpt_idx == -1) {
// The 'e' ends the whole-number part, so check its last group:
if (badLeastGroup())
return false;
if (last_separator_idx == -1) {
// Check distance from the beginning of the digits:
if (start_of_digits_idx == -1 || grouping.first > digitsInGroup
|| digitsInGroup >= grouping.least + grouping.first) {
return false;
}
} else {
// Check distance from the last separator:
if (digitsInGroup != grouping.higher)
return false;
}
last_separator_idx = idx;
digitsInGroup = 0;
} else if (mode != IntegerMode && (out == '.' || idx == exponent_idx)
&& last_separator_idx != -1) {
// Were there enough digits since the last group separator?
if (digitsInGroup != grouping.least)
last_separator_idx = -1; // Process no separators after this
} else if (number_options.testFlag(QLocale::RejectTrailingZeroesAfterDot)) {
// In a fractional part, a 0 just before the exponent is trailing:
if (last == '0')
return false;
// stop processing separators
last_separator_idx = -1;
}
} else if (out == ',') {
return false;
if (number_options.testFlag(QLocale::RejectGroupSeparator))
return false;
// Don't allow group chars after the decimal point or exponent
if (decpt_idx != -1 || exponent_idx != -1)
return false;
if (last_separator_idx == -1) {
// Check size of most significant group
if (start_of_digits_idx == -1 || grouping.first > digitsInGroup
|| digitsInGroup >= grouping.least + grouping.first) {
return false;
}
} else {
// Check size of group between two separators:
if (digitsInGroup != grouping.higher)
return false;
}
last_separator_idx = idx;
digitsInGroup = 0;
} else if (isAsciiDigit(out)) {
if (out == '0' && number_options.testFlag(QLocale::RejectLeadingZeroInExponent)
&& exponent_idx != -1 && !tokens.done() && !isAsciiDigit(last)) {
// After the exponent there can only be '+', '-' or digits. If
// we find a '0' directly after some non-digit, then that is a
// leading zero, acceptable only if it is the whole exponent.
return false;
}
if (start_of_digits_idx == -1)
start_of_digits_idx = idx;
++digitsInGroup;
}
// else: nothing special to do.
last = out;
if (out != ',') // Leave group separators out of the result.
result->append(out);
}
if (!number_options.testFlag(QLocale::RejectGroupSeparator) && last_separator_idx != -1) {
// Were there enough digits since the last group separator?
if (digitsInGroup != grouping.least)
if (!number_options.testFlag(QLocale::RejectGroupSeparator)) {
// If this is the end of the whole-part, check least significant group:
if (decpt_idx == -1 && exponent_idx == -1 && badLeastGroup())
return false;
}

8
src/corelib/text/qlocale_p.h
View File

@ -274,11 +274,11 @@ public:
enum NumberMode { IntegerMode, DoubleStandardMode, DoubleScientificMode };
struct GroupSizes
struct GroupSizes // Numbers of digits in various groups:
{
int first = 0;
int higher = 0;
int least = 0;
int first = 0; // Min needed before the separator, when there's only one.
int higher = 0; // Each group between separators.
int least = 0; // Least significant, when any separators appear.
bool isValid() const { return least > 0 && higher > first && first > 0; }
};