Fix digit grouping when digits are surrogat pairs

This is a follow-up to commit ed2b110b6add650954dc102a0317c14ff826c677
to fix indexing errors. Added the test that should have accompanied
that commit, which found some bugs, and refined the Indian number
formatting test (on which it's based).

Make variable i in the loops that insert grouping characters in a
number be consistently a *character* offset - which, when each digit
is a surrogate pair, isn't the same as an index into the
QString. Apply the needed scaling when indexing with it, not when
setting it or decrementing it. Don't assume the separator has the same
width as a digit.

Differences in index no longer give the number of digits between two
points in a string, so actively track how many digits we've seen in a
group when converting a numeric string to the C locale. Partially
cleaned up the code for that in the process (more shall follow when I
sort out digit grouping properly, without special-casing India).

Change-Id: I13d0f24efa26e599dfefb5733e062088fa56d375
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

1 files changed, 30 insertions, 24 deletions

diff --git a/src/corelib/text/qlocale.cpp b/src/corelib/text/qlocale.cpp
index 258e90a06c..467fd31ab6 100644
--- a/src/corelib/text/qlocale.cpp
+++ b/src/corelib/text/qlocale.cpp
@@ -3712,15 +3712,16 @@ QT_WARNING_POP
     uint cnt_thousand_sep = 0;
     if (base == 10) {
         if (flags & ThousandsGroup) {
-            for (int i = num_str.length() / digitWidth - 3; i > 0; i -= 3 * digitWidth) {
-                num_str.insert(i, group);
+            for (int i = num_str.length() / digitWidth - 3; i > 0; i -= 3) {
+                num_str.insert(i * digitWidth, group);
                 ++cnt_thousand_sep;
             }
         } else if (flags & IndianNumberGrouping) {
-            if (num_str.length() > 3 * digitWidth)
-                num_str.insert(num_str.length() - 3 * digitWidth , group);
-            for (int i = num_str.length() - 6 * digitWidth; i > 0; i -= 2 * digitWidth) {
-                num_str.insert(i, group);
+            const int size = num_str.length();
+            if (size > 3 * digitWidth)
+                num_str.insert(size - 3 * digitWidth , group);
+            for (int i = size / digitWidth - 5; i > 0; i -= 2) {
+                num_str.insert(i * digitWidth, group);
                 ++cnt_thousand_sep;
             }
         }
@@ -3807,15 +3808,16 @@ QString QLocaleData::unsLongLongToString(const QString &zero, const QString &gro
     uint cnt_thousand_sep = 0;
     if (base == 10) {
         if (flags & ThousandsGroup) {
-            for (int i = num_str.length() - 3 * digitWidth; i > 0; i -= 3 * digitWidth) {
-                num_str.insert(i, group);
+            for (int i = num_str.length() / digitWidth - 3; i > 0; i -= 3) {
+                num_str.insert(i * digitWidth, group);
                 ++cnt_thousand_sep;
             }
         } else if (flags & IndianNumberGrouping) {
-            if (num_str.length() > 3 * digitWidth)
-                num_str.insert(num_str.length() - 3 * digitWidth , group);
-            for (int i = num_str.length() - 6 * digitWidth; i > 0; i -= 2 * digitWidth) {
-                num_str.insert(i, group);
+            const int size = num_str.length();
+            if (size > 3 * digitWidth)
+                num_str.insert(size - 3 * digitWidth , group);
+            for (int i = size / digitWidth - 5; i > 0; i -= 2) {
+                num_str.insert(i * digitWidth, group);
                 ++cnt_thousand_sep;
             }
         }
@@ -3886,6 +3888,8 @@ bool QLocaleData::numberToCLocale(QStringView s, QLocale::NumberOptions number_o
     auto length = s.size();
     decltype(length) idx = 0;
 
+    const int leadingGroupWidth = (m_country_id == QLocale::India ? 2 : 3);
+    int digitsInGroup = 0;
     int group_cnt = 0; // counts number of group chars
     int decpt_idx = -1;
     int last_separator_idx = -1;
@@ -3937,26 +3941,26 @@ bool QLocaleData::numberToCLocale(QStringView s, QLocale::NumberOptions number_o
         if (!(number_options & QLocale::RejectGroupSeparator)) {
             if (start_of_digits_idx == -1 && out >= '0' && out <= '9') {
                 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)
                     return false;
 
-                // check distance from the last separator or from the beginning of the digits
-                // ### FIXME: Some locales allow other groupings!
-                // See https://en.wikipedia.org/wiki/Thousands_separator
-                if (m_country_id == QLocale::India) {
-                    if (last_separator_idx != -1 && idx - last_separator_idx != 3)
+                if (last_separator_idx == -1) {
+                    if (start_of_digits_idx == -1 || digitsInGroup > leadingGroupWidth)
+                        return false;
+                } else {
+                    // check distance from the last separator or from the beginning of the digits
+                    // ### FIXME: Some locales allow other groupings!
+                    // See https://en.wikipedia.org/wiki/Thousands_separator
+                    if (digitsInGroup != leadingGroupWidth)
                         return false;
-                } else if (last_separator_idx != -1 && idx - last_separator_idx != 4)
-                    return false;
-                if (last_separator_idx == -1
-                    && (start_of_digits_idx == -1 || idx - start_of_digits_idx > 3)) {
-                    return false;
                 }
 
                 last_separator_idx = idx;
                 ++group_cnt;
+                digitsInGroup = 0;
 
                 // don't add the group separator
                 idx += in.size();
@@ -3965,11 +3969,13 @@ bool QLocaleData::numberToCLocale(QStringView s, QLocale::NumberOptions number_o
                 // check distance from the last separator
                 // ### FIXME: Some locales allow other groupings!
                 // See https://en.wikipedia.org/wiki/Thousands_separator
-                if (last_separator_idx != -1 && idx - last_separator_idx != 4)
+                if (last_separator_idx != -1 && digitsInGroup != 3)
                     return false;
 
                 // stop processing separators
                 last_separator_idx = -1;
+            } else if (out >= '0' && out <= '9') {
+                digitsInGroup++;
             }
         }
 
@@ -3983,7 +3989,7 @@ bool QLocaleData::numberToCLocale(QStringView s, QLocale::NumberOptions number_o
         if (last_separator_idx + 1 == idx)
             return false;
         // were there enough digits since the last separator?
-        if (last_separator_idx != -1 && idx - last_separator_idx != 4)
+        if (last_separator_idx != -1 && digitsInGroup != 3)
             return false;
     }