diff --git a/core/src/kmx/kmx_plus.cpp b/core/src/kmx/kmx_plus.cpp index b02fec77d8..a0da603d95 100644 --- a/core/src/kmx/kmx_plus.cpp +++ b/core/src/kmx/kmx_plus.cpp @@ -1072,16 +1072,16 @@ COMP_KMXPLUS_USET_Helper::setUset(const COMP_KMXPLUS_USET *newUset) { return is_valid; } -USet::USet(const COMP_KMXPLUS_USET_RANGE *newRange, size_t newCount) { +SimpleUSet::SimpleUSet(const COMP_KMXPLUS_USET_RANGE *newRange, size_t newCount) { for (size_t i = 0; i < newCount; i++) { ranges.emplace_back(newRange[i].start, newRange[i].end); } } -USet::USet() { +SimpleUSet::SimpleUSet() { } -bool USet::contains(km_kbp_usv ch) const { +bool SimpleUSet::contains(km_kbp_usv ch) const { for (const auto &range : ranges) { if (range.start <= ch && range.end >= ch) { return true; @@ -1091,7 +1091,7 @@ bool USet::contains(km_kbp_usv ch) const { } bool -USet::valid() const { +SimpleUSet::valid() const { // double check for (const auto &range : ranges) { if (!Uni_IsValid(range.start, range.end)) { @@ -1103,7 +1103,7 @@ USet::valid() const { } void -USet::dump() const { +SimpleUSet::dump() const { DebugLog(" - USet size=%d", ranges.size()); for (const auto &range : ranges) { if (range.start == range.end) { @@ -1114,14 +1114,14 @@ USet::dump() const { } } -USet +SimpleUSet COMP_KMXPLUS_USET_Helper::getUset(KMXPLUS_USET i) const { if (!valid() || i >= uset->usetCount) { assert(false); - return USet(nullptr, 0); // empty set + return SimpleUSet(nullptr, 0); // empty set } auto &set = usets[i]; - return USet(getRange(set.range), set.count); + return SimpleUSet(getRange(set.range), set.count); } const COMP_KMXPLUS_USET_RANGE * diff --git a/core/src/kmx/kmx_plus.h b/core/src/kmx/kmx_plus.h index 2962ac8789..12b2cd44e7 100644 --- a/core/src/kmx/kmx_plus.h +++ b/core/src/kmx/kmx_plus.h @@ -694,14 +694,16 @@ struct COMP_KMXPLUS_USET_RANGE { }; /** - * represents one of the uset elements + * represents one of the uset elements. + * TODO-LDML: replace this with a real icu::UnicodeSet? or at least + * a function producing the same? */ -class USet { +class SimpleUSet { public: /** construct a set over the specified range. Data is copied. */ - USet(const COMP_KMXPLUS_USET_RANGE* newStart, size_t newCount); + SimpleUSet(const COMP_KMXPLUS_USET_RANGE* newStart, size_t newCount); /** empty set */ - USet(); + SimpleUSet(); /** true if the uset contains this char */ bool contains(km_kbp_usv ch) const; /** debugging */ @@ -721,7 +723,7 @@ public: bool setUset(const COMP_KMXPLUS_USET *newUset); inline bool valid() const { return is_valid; } - USet getUset(KMXPLUS_USET list) const; + SimpleUSet getUset(KMXPLUS_USET list) const; const COMP_KMXPLUS_USET_RANGE *getRange(KMX_DWORD index) const; private: diff --git a/core/src/ldml/ldml_transforms.cpp b/core/src/ldml/ldml_transforms.cpp index f1dc2d6916..64cc206053 100644 --- a/core/src/ldml/ldml_transforms.cpp +++ b/core/src/ldml/ldml_transforms.cpp @@ -11,20 +11,6 @@ #include #include "kmx/kmx_xstring.h" -#if defined(HAVE_ICU4C) -// TODO-LDML: Needed this for some compiler warnings -#define U_FALLTHROUGH -#include "unicode/uniset.h" -#include "unicode/usetiter.h" -#include "unicode/unistr.h" -#include "unicode/regex.h" -#include "unicode/utext.h" -#include "unicode/utypes.h" -#else -#error icu4c is required for this code -#endif - - #ifndef assert #define assert(x) // TODO-LDML #endif @@ -49,7 +35,7 @@ namespace ldml { #define DebugTran(msg, ...) #endif -element::element(const USet &new_u, KMX_DWORD new_flags) +element::element(const SimpleUSet &new_u, KMX_DWORD new_flags) : chr(), uset(new_u), flags((new_flags & ~LDML_ELEM_FLAGS_TYPE) | LDML_ELEM_FLAGS_TYPE_USET) { } @@ -204,7 +190,7 @@ element_list::load(const kmx::kmx_plus &kplus, kmx::KMXPLUS_ELEM id) { km_kbp_usv ch = e.element; emplace_back(ch, flags); // char } else if (type == LDML_ELEM_FLAGS_TYPE_USET) { - // need to load a USet + // need to load a SimpleUSet auto u = kplus.usetHelper.getUset(e.element); if (!u.valid()) { DebugLog("Error, invalid UnicodeSet at element %d", (int)i); @@ -406,25 +392,36 @@ reorder_group::apply(std::u32string &str) const { return applied; } +transform_entry::transform_entry(const transform_entry &other) : + fFrom(other.fFrom), fTo(other.fTo), fFromPattern(nullptr) { + if (other.fFromPattern) { + // clone pattern + fFromPattern.reset(other.fFromPattern->clone()); + } +} + transform_entry::transform_entry(const std::u32string &from, const std::u32string &to) : fFrom(from), fTo(to) { + assert(!fFrom.empty()); // TODO-LDML: should not happen? + + if (!fFrom.empty()) { + const std::u16string patstr = km::kbp::kmx::u32string_to_u16string(fFrom); + UErrorCode status = U_ZERO_ERROR; + /* const */ icu::UnicodeString patustr = icu::UnicodeString(patstr.data(), (int32_t)patstr.length()); + // add '$' to match to end + patustr.append(u'$'); + fFromPattern.reset(icu::RegexPattern::compile(patustr, 0, status)); + assert(U_SUCCESS(status)); // TODO-LDML: may be best to propagate status up ^^ + } } size_t transform_entry::match(const std::u32string &input) const { - // TODO-LDML: simple approach, new regex every time + assert(fFromPattern); // TODO-LDML: Really? can't go from u32 to UnicodeString? - - const std::u16string patstr = km::kbp::kmx::u32string_to_u16string(fFrom); - UErrorCode status = U_ZERO_ERROR; - /* const */ icu::UnicodeString patustr = icu::UnicodeString(patstr.data(), (int32_t)patstr.length()); - // add '$' to match to end - patustr.append(u'$'); - std::unique_ptr pattern(icu::RegexPattern::compile(patustr, 0, status)); - assert(U_SUCCESS(status)); - + UErrorCode status = U_ZERO_ERROR; const std::u16string matchstr = km::kbp::kmx::u32string_to_u16string(input); icu::UnicodeString matchustr = icu::UnicodeString(matchstr.data(), (int32_t)matchstr.length()); - std::unique_ptr matcher(pattern->matcher(matchustr, status)); + std::unique_ptr matcher(fFromPattern->matcher(matchustr, status)); assert(U_SUCCESS(status)); if (!matcher->find(status)) { // i.e. matches somewhere, in this case at end of str @@ -447,58 +444,53 @@ transform_entry::match(const std::u32string &input) const { std::u32string transform_entry::apply(const std::u32string &input, size_t matchLen) const { // TODO-LDML: and if you thought the previous function was suboptimal, - // TODO-LDML: now we're going to do it all again! - // TODO-LDML: simple approach, new regex every time + // TODO-LDML: We should cache the RegexMatcher from the previous call. // TODO-LDML: Really? can't go from u32 to UnicodeString? - const std::u16string patstr = km::kbp::kmx::u32string_to_u16string(fFrom); - UErrorCode status = U_ZERO_ERROR; - /* const */ icu::UnicodeString patustr = icu::UnicodeString(patstr.data(), (int32_t)patstr.length()); - // add '$' to match to end - patustr.append(u'$'); - std::unique_ptr pattern(icu::RegexPattern::compile(patustr, 0, status)); - assert(U_SUCCESS(status)); + assert(fFromPattern); + // TODO-LDML: simple approach, new regex every time + // TODO-LDML: Really? can't go from u32 to UnicodeString? + UErrorCode status = U_ZERO_ERROR; // we know the matchLen so we can slice the string… - const std::u16string matchstr = km::kbp::kmx::u32string_to_u16string(input.substr(input.length()-matchLen, matchLen)); - icu::UnicodeString matchustr = icu::UnicodeString(matchstr.data(), (int32_t)matchstr.length()); - std::unique_ptr matcher(pattern->matcher(matchustr, status)); + // Note: the matchstr here (unlike in transform_entry::match) is sliced so that + // it only includes the matched portion. This way, when changed it's suitable as the + // output string. + const std::u16string matchstr = km::kbp::kmx::u32string_to_u16string(input.substr(input.length() - matchLen, matchLen)); + icu::UnicodeString matchustr = icu::UnicodeString(matchstr.data(), (int32_t)matchstr.length()); + std::unique_ptr matcher(fFromPattern->matcher(matchustr, status)); assert(U_SUCCESS(status)); - // assert(matcher->find(status)); // it better match + // now, do the replace const std::u16string rstr = km::kbp::kmx::u32string_to_u16string(fTo); - icu::UnicodeString rustr = icu::UnicodeString(rstr.data(), (int32_t)rstr.length()); + icu::UnicodeString rustr = icu::UnicodeString(rstr.data(), (int32_t)rstr.length()); + // This replace will apply $1, $2 etc. TODO-LDML it will NOT handle mapFrom or mapTo. icu::UnicodeString output = matcher->replaceFirst(rustr, status); assert(U_SUCCESS(status)); - // if (!matcher->find(status)) { // i.e. matches somewhere, in this case at end of str - // return 0; // and tear everything down - // } + if (output.length() == 0) { + return std::u32string(); // special case of a zero length output (such as delete) + } - - - // // TODO-LDML: this is UTF-16 len, not UTF-32 len!! - // // auto matchLen = matcher->end64(status) - matcher->start64(status); - // // TODO-LDML: if we had an underlying UText this would be simpler. - // auto matchStart = matcher->start64(status); - // auto matchEnd = matcher->end64(status); - // // extract.. - // const icu::UnicodeString substr = matchustr.tempSubStringBetween((int32_t)matchStart, (int32_t)matchEnd); - // // preflight to UTF-32 to get length - // auto matchLen = substr.toUTF32(nullptr, 0, status); - - // return matchLen; - assert(U_SUCCESS(status)); + // TODO-LDML: All we are trying to do is to extract the output string. Probably too many steps. UErrorCode preflightStatus = U_ZERO_ERROR; - auto out32len = output.toUTF32(nullptr, 0, preflightStatus); - char32_t *s = new char32_t[out32len+1]; - output.toUTF32((UChar32*)s, out32len+1, status); - std::u32string out32(s, out32len); + // calculate how big the buffer is + auto out32len = output.toUTF32(nullptr, 0, preflightStatus); // preflightStatus will be an err, because we know the buffer overruns zero bytes + // allocate + char32_t *s = new char32_t[out32len + 1]; + assert(s != nullptr); + // convert + output.toUTF32((UChar32 *)s, out32len + 1, status); assert(U_SUCCESS(status)); + // now, build a u32string + std::u32string out32(s, out32len); + // clean up buffer + delete [] s; return out32; } any_group::any_group(const transform_group &g) : type(any_group_type::transform), transform(g), reorder() { } + any_group::any_group(const reorder_group &g) : type(any_group_type::reorder), transform(), reorder(g) { } diff --git a/core/src/ldml/ldml_transforms.hpp b/core/src/ldml/ldml_transforms.hpp index 6296399f2b..56a4117c4d 100644 --- a/core/src/ldml/ldml_transforms.hpp +++ b/core/src/ldml/ldml_transforms.hpp @@ -14,11 +14,23 @@ #include #include +#if !defined(HAVE_ICU4C) +#error icu4c is required for this code +#endif + +#define U_FALLTHROUGH +#include "unicode/utypes.h" +#include "unicode/uniset.h" +#include "unicode/usetiter.h" +#include "unicode/unistr.h" +#include "unicode/regex.h" +#include "unicode/utext.h" + namespace km { namespace kbp { namespace ldml { -using km::kbp::kmx::USet; +using km::kbp::kmx::SimpleUSet; /** * Type of a group @@ -33,12 +45,11 @@ enum any_group_type { */ class element { public: - /** construct from a USet */ - element(const USet &u, KMX_DWORD flags); + /** construct from a SimpleUSet */ + element(const SimpleUSet &u, KMX_DWORD flags); /** construct from a single char */ element(km_kbp_usv ch, KMX_DWORD flags); - - /** @returns true if a USet type */ + /** @returns true if a SimpleUSet type */ bool is_uset() const; /** @returns true if prebase bit set*/ bool is_prebase() const; @@ -58,7 +69,7 @@ public: private: // TODO-LDML: support multi-char strings? const km_kbp_usv chr; - const USet uset; + const SimpleUSet uset; const KMX_DWORD flags; }; @@ -67,6 +78,7 @@ private: */ class transform_entry { public: + transform_entry(const transform_entry &other); transform_entry( const std::u32string &from, const std::u32string &to @@ -84,8 +96,9 @@ public: std::u32string apply(const std::u32string &input, size_t matchLen) const; private: - const std::u32string fFrom; // TODO-LDML: regex + const std::u32string fFrom; const std::u32string fTo; + std::unique_ptr fFromPattern; }; /** diff --git a/core/tests/unit/ldml/ldml_test_source.cpp b/core/tests/unit/ldml/ldml_test_source.cpp index 1caae7de60..673f692f1e 100644 --- a/core/tests/unit/ldml/ldml_test_source.cpp +++ b/core/tests/unit/ldml/ldml_test_source.cpp @@ -35,6 +35,7 @@ #if defined(HAVE_ICU4C) // TODO-LDML: Needed this for some compiler warnings #define U_FALLTHROUGH +#include "unicode/utypes.h" #include "unicode/uniset.h" #include "unicode/usetiter.h" #else diff --git a/core/tests/unit/ldml/test_kmx_plus.cpp b/core/tests/unit/ldml/test_kmx_plus.cpp index 74961f797e..7f158dd1c3 100644 --- a/core/tests/unit/ldml/test_kmx_plus.cpp +++ b/core/tests/unit/ldml/test_kmx_plus.cpp @@ -152,12 +152,12 @@ int test_uset() { {0x127, 0x127} // [ħ] }; - USet u0(&r[0], 2); + SimpleUSet u0(&r[0], 2); assert_equal(u0.contains(0x62), true); // b assert_equal(u0.contains(0x41), false); // A assert_equal(u0.contains(0x127), true); // ħ - USet uempty; + SimpleUSet uempty; assert_equal(uempty.contains(0x62), false); assert_equal(uempty.contains(0x127), false); diff --git a/core/tests/unit/ldml/test_transforms.cpp b/core/tests/unit/ldml/test_transforms.cpp index d0ab45e1ce..db51b19ac3 100644 --- a/core/tests/unit/ldml/test_transforms.cpp +++ b/core/tests/unit/ldml/test_transforms.cpp @@ -166,7 +166,7 @@ test_reorder_standalone() { COMP_KMXPLUS_USET_RANGE(0x1A75, 0x1A79)}; const COMP_KMXPLUS_USET_USET usets[] = {{0, 1, 0xFFFFFFFF}}; const COMP_KMXPLUS_USET_USET &toneMarksUset = usets[0]; - const USet toneMarks(&ranges[toneMarksUset.range], toneMarksUset.count); + const SimpleUSet toneMarks(&ranges[toneMarksUset.range], toneMarksUset.count); // validate that the range [1A75, 1A79] matches assert_equal(toneMarks.contains(0x1A76), true); assert_equal(toneMarks.contains(0x1A60), false);