From fb3ff21caf5dc582885bc452ce5398de5da8ebc3 Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Fri, 9 Feb 2018 05:47:49 +0000 Subject: [PATCH] ICU-13574 Switching memory strategy to allocate objects as fields in the main class instead of on the heap. X-SVN-Rev: 40877 --- icu4c/source/i18n/numparse_decimal.h | 2 ++ icu4c/source/i18n/numparse_impl.cpp | 32 +++++++++---------- icu4c/source/i18n/numparse_impl.h | 20 ++++++++++-- icu4c/source/i18n/numparse_symbols.h | 22 ++++++++++--- icu4c/source/i18n/numparse_unisets.cpp | 17 +++++----- .../source/test/intltest/numbertest_parse.cpp | 21 ++++++------ 6 files changed, 72 insertions(+), 42 deletions(-) diff --git a/icu4c/source/i18n/numparse_decimal.h b/icu4c/source/i18n/numparse_decimal.h index b7bda16f589..9423a7786c9 100644 --- a/icu4c/source/i18n/numparse_decimal.h +++ b/icu4c/source/i18n/numparse_decimal.h @@ -17,6 +17,8 @@ using ::icu::number::impl::Grouper; class DecimalMatcher : public NumberParseMatcher, public UMemory { public: + DecimalMatcher() = default; // WARNING: Leaves the object in an unusable state + DecimalMatcher(const DecimalFormatSymbols& symbols, const Grouper& grouper, parse_flags_t parseFlags); diff --git a/icu4c/source/i18n/numparse_impl.cpp b/icu4c/source/i18n/numparse_impl.cpp index 99d19df455f..1df9c56b43b 100644 --- a/icu4c/source/i18n/numparse_impl.cpp +++ b/icu4c/source/i18n/numparse_impl.cpp @@ -29,7 +29,7 @@ NumberParserImpl::createSimpleParser(const Locale& locale, const UnicodeString& auto* parser = new NumberParserImpl(parseFlags, true); DecimalFormatSymbols symbols(locale, status); - IgnorablesMatcher* ignorables = new IgnorablesMatcher(unisets::DEFAULT_IGNORABLES); + parser->fLocalMatchers.ignorables = {unisets::DEFAULT_IGNORABLES}; // MatcherFactory factory = new MatcherFactory(); // factory.currency = Currency.getInstance("USD"); @@ -45,13 +45,13 @@ NumberParserImpl::createSimpleParser(const Locale& locale, const UnicodeString& Grouper grouper = Grouper::forStrategy(UNUM_GROUPING_AUTO); grouper.setLocaleData(patternInfo, locale); - parser->addAndAdoptMatcher(ignorables); - parser->addAndAdoptMatcher(new DecimalMatcher(symbols, grouper, parseFlags)); - parser->addAndAdoptMatcher(new MinusSignMatcher(symbols, false)); - parser->addAndAdoptMatcher(new PlusSignMatcher(symbols, false)); - parser->addAndAdoptMatcher(new PercentMatcher(symbols)); - parser->addAndAdoptMatcher(new PermilleMatcher(symbols)); - parser->addAndAdoptMatcher(new NanMatcher(symbols)); + parser->addMatcher(parser->fLocalMatchers.ignorables); + parser->addMatcher(parser->fLocalMatchers.decimal = {symbols, grouper, parseFlags}); + parser->addMatcher(parser->fLocalMatchers.minusSign = {symbols, false}); + parser->addMatcher(parser->fLocalMatchers.plusSign = {symbols, false}); + parser->addMatcher(parser->fLocalMatchers.percent = {symbols}); + parser->addMatcher(parser->fLocalMatchers.permille = {symbols}); + parser->addMatcher(parser->fLocalMatchers.nan = {symbols}); // parser.addMatcher(ScientificMatcher.getInstance(symbols, grouper, parseFlags)); // parser.addMatcher(CurrencyTrieMatcher.getInstance(locale)); // parser.addMatcher(new RequireNumberMatcher()); @@ -65,16 +65,15 @@ NumberParserImpl::NumberParserImpl(parse_flags_t parseFlags, bool computeLeads) } NumberParserImpl::~NumberParserImpl() { - for (int32_t i = 0; i < fNumMatchers; i++) { - delete (fMatchers[i]); - if (fComputeLeads) { + if (fComputeLeads) { + for (int32_t i = 0; i < fNumMatchers; i++) { delete (fLeads[i]); } } fNumMatchers = 0; } -void NumberParserImpl::addAndAdoptMatcher(const NumberParseMatcher* matcher) { +void NumberParserImpl::addMatcher(const NumberParseMatcher& matcher) { if (fNumMatchers + 1 > fMatchers.getCapacity()) { fMatchers.resize(fNumMatchers * 2, fNumMatchers); if (fComputeLeads) { @@ -84,10 +83,10 @@ void NumberParserImpl::addAndAdoptMatcher(const NumberParseMatcher* matcher) { } } - fMatchers[fNumMatchers] = matcher; + fMatchers[fNumMatchers] = &matcher; if (fComputeLeads) { - fLeads[fNumMatchers] = matcher->getLeadCodePoints(); + fLeads[fNumMatchers] = matcher.getLeadCodePoints(); } fNumMatchers++; @@ -102,9 +101,8 @@ void NumberParserImpl::parse(const UnicodeString& input, bool greedy, ParsedNumb return parse(input, 0, greedy, result, status); } -void -NumberParserImpl::parse(const UnicodeString& input, int32_t start, bool greedy, ParsedNumber& result, - UErrorCode& status) const { +void NumberParserImpl::parse(const UnicodeString& input, int32_t start, bool greedy, ParsedNumber& result, + UErrorCode& status) const { U_ASSERT(fFrozen); // TODO: Check start >= 0 and start < input.length() StringSegment segment(input, fParseFlags); diff --git a/icu4c/source/i18n/numparse_impl.h b/icu4c/source/i18n/numparse_impl.h index adb92946894..105f8c71abe 100644 --- a/icu4c/source/i18n/numparse_impl.h +++ b/icu4c/source/i18n/numparse_impl.h @@ -8,6 +8,8 @@ #define __NUMPARSE_IMPL_H__ #include "numparse_types.h" +#include "numparse_decimal.h" +#include "numparse_symbols.h" #include "unicode/uniset.h" U_NAMESPACE_BEGIN namespace numparse { @@ -15,10 +17,12 @@ namespace impl { class NumberParserImpl { public: + ~NumberParserImpl(); + static NumberParserImpl* createSimpleParser(const Locale& locale, const UnicodeString& patternString, parse_flags_t parseFlags, UErrorCode& status); - void addAndAdoptMatcher(const NumberParseMatcher* matcher); + void addMatcher(const NumberParseMatcher& matcher); void freeze(); @@ -38,9 +42,19 @@ class NumberParserImpl { bool fComputeLeads; bool fFrozen = false; - NumberParserImpl(parse_flags_t parseFlags, bool computeLeads); + // WARNING: All of these matchers start in an uninitialized state. + // You must use an assignment operator on them before using. + struct { + IgnorablesMatcher ignorables; + MinusSignMatcher minusSign; + NanMatcher nan; + PercentMatcher percent; + PermilleMatcher permille; + PlusSignMatcher plusSign; + DecimalMatcher decimal; + } fLocalMatchers; - ~NumberParserImpl(); + NumberParserImpl(parse_flags_t parseFlags, bool computeLeads); void parseGreedyRecursive(StringSegment& segment, ParsedNumber& result, UErrorCode& status) const; diff --git a/icu4c/source/i18n/numparse_symbols.h b/icu4c/source/i18n/numparse_symbols.h index 4a17d67a44d..c8ba913911f 100644 --- a/icu4c/source/i18n/numparse_symbols.h +++ b/icu4c/source/i18n/numparse_symbols.h @@ -17,6 +17,8 @@ namespace impl { class SymbolMatcher : public NumberParseMatcher, public UMemory { public: + SymbolMatcher() = default; // WARNING: Leaves the object in an unusable state + const UnicodeSet* getSet(); bool match(StringSegment& segment, ParsedNumber& result, UErrorCode& status) const override; @@ -37,7 +39,9 @@ class SymbolMatcher : public NumberParseMatcher, public UMemory { class IgnorablesMatcher : public SymbolMatcher { public: - explicit IgnorablesMatcher(unisets::Key key); + IgnorablesMatcher() = default; // WARNING: Leaves the object in an unusable state + + IgnorablesMatcher(unisets::Key key); bool isFlexible() const override; @@ -50,6 +54,8 @@ class IgnorablesMatcher : public SymbolMatcher { class MinusSignMatcher : public SymbolMatcher { public: + MinusSignMatcher() = default; // WARNING: Leaves the object in an unusable state + MinusSignMatcher(const DecimalFormatSymbols& dfs, bool allowTrailing); protected: @@ -64,7 +70,9 @@ class MinusSignMatcher : public SymbolMatcher { class NanMatcher : public SymbolMatcher { public: - explicit NanMatcher(const DecimalFormatSymbols& dfs); + NanMatcher() = default; // WARNING: Leaves the object in an unusable state + + NanMatcher(const DecimalFormatSymbols& dfs); const UnicodeSet* getLeadCodePoints() const override; @@ -77,7 +85,9 @@ class NanMatcher : public SymbolMatcher { class PercentMatcher : public SymbolMatcher { public: - explicit PercentMatcher(const DecimalFormatSymbols& dfs); + PercentMatcher() = default; // WARNING: Leaves the object in an unusable state + + PercentMatcher(const DecimalFormatSymbols& dfs); void postProcess(ParsedNumber& result) const override; @@ -90,7 +100,9 @@ class PercentMatcher : public SymbolMatcher { class PermilleMatcher : public SymbolMatcher { public: - explicit PermilleMatcher(const DecimalFormatSymbols& dfs); + PermilleMatcher() = default; // WARNING: Leaves the object in an unusable state + + PermilleMatcher(const DecimalFormatSymbols& dfs); void postProcess(ParsedNumber& result) const override; @@ -103,6 +115,8 @@ class PermilleMatcher : public SymbolMatcher { class PlusSignMatcher : public SymbolMatcher { public: + PlusSignMatcher() = default; // WARNING: Leaves the object in an unusable state + PlusSignMatcher(const DecimalFormatSymbols& dfs, bool allowTrailing); protected: diff --git a/icu4c/source/i18n/numparse_unisets.cpp b/icu4c/source/i18n/numparse_unisets.cpp index a7d1fbdba26..f259f7a6467 100644 --- a/icu4c/source/i18n/numparse_unisets.cpp +++ b/icu4c/source/i18n/numparse_unisets.cpp @@ -19,7 +19,7 @@ using namespace icu::numparse::impl::unisets; namespace { -UnicodeSet* gUnicodeSets[COUNT] = {}; +static UnicodeSet* gUnicodeSets[COUNT] = {}; UnicodeSet* computeUnion(Key k1, Key k2) { UnicodeSet* result = new UnicodeSet(); @@ -46,7 +46,7 @@ UnicodeSet* computeUnion(Key k1, Key k2, Key k3) { icu::UInitOnce gNumberParseUniSetsInitOnce = U_INITONCE_INITIALIZER; -UBool U_CALLCONV cleanupNumberParseUnitSets() { +UBool U_CALLCONV cleanupNumberParseUniSets() { for (int32_t i = 0; i < COUNT; i++) { delete gUnicodeSets[i]; gUnicodeSets[i] = nullptr; @@ -54,8 +54,8 @@ UBool U_CALLCONV cleanupNumberParseUnitSets() { return TRUE; } -void U_CALLCONV initNumberParseUniSets(UErrorCode &status) { - ucln_i18n_registerCleanup(UCLN_I18N_NUMPARSE_UNISETS, cleanupNumberParseUnitSets); +void U_CALLCONV initNumberParseUniSets(UErrorCode& status) { + ucln_i18n_registerCleanup(UCLN_I18N_NUMPARSE_UNISETS, cleanupNumberParseUniSets); #define NEW_UNISET(pattern, status) new UnicodeSet(UnicodeString(pattern), status) gUnicodeSets[EMPTY] = new UnicodeSet(); @@ -68,7 +68,7 @@ void U_CALLCONV initNumberParseUniSets(UErrorCode &status) { gUnicodeSets[WHITESPACE] = NEW_UNISET(u"[[:Zs:][\\u0009]]", status); gUnicodeSets[DEFAULT_IGNORABLES] = computeUnion(BIDI, WHITESPACE); - gUnicodeSets[STRICT_IGNORABLES] = gUnicodeSets[BIDI]; + gUnicodeSets[STRICT_IGNORABLES] = new UnicodeSet(*gUnicodeSets[BIDI]); // TODO: Re-generate these sets from the UCD. They probably haven't been updated in a while. gUnicodeSets[COMMA] = NEW_UNISET(u"[,،٫、︐︑﹐﹑，､]", status); @@ -76,7 +76,8 @@ void U_CALLCONV initNumberParseUniSets(UErrorCode &status) { gUnicodeSets[PERIOD] = NEW_UNISET(u"[.․。︒﹒．｡]", status); gUnicodeSets[STRICT_PERIOD] = NEW_UNISET(u"[.․﹒．｡]", status); gUnicodeSets[OTHER_GROUPING_SEPARATORS] = NEW_UNISET( - u"['٬‘’＇\\u0020\\u00A0\\u2000-\\u200A\\u202F\\u205F\\u3000]", status); + u"['٬‘’＇\\u0020\\u00A0\\u2000-\\u200A\\u202F\\u205F\\u3000]", + status); gUnicodeSets[ALL_SEPARATORS] = computeUnion(COMMA, PERIOD, OTHER_GROUPING_SEPARATORS); gUnicodeSets[STRICT_ALL_SEPARATORS] = computeUnion( STRICT_COMMA, STRICT_PERIOD, OTHER_GROUPING_SEPARATORS); @@ -89,8 +90,8 @@ void U_CALLCONV initNumberParseUniSets(UErrorCode &status) { gUnicodeSets[INFINITY] = NEW_UNISET(u"[∞]", status); gUnicodeSets[DIGITS] = NEW_UNISET(u"[:digit:]", status); - gUnicodeSets[NAN_LEAD] = NEW_UNISET( - u"[NnТтmeՈոс¤НнчTtsҳ\u975e\u1002\u0e9a\u10d0\u0f68\u0644\u0646]", status); + gUnicodeSets[NAN_LEAD] = NEW_UNISET(u"[NnТтmeՈոс¤НнчTtsҳ\u975e\u1002\u0e9a\u10d0\u0f68\u0644\u0646]", + status); gUnicodeSets[SCIENTIFIC_LEAD] = NEW_UNISET(u"[Ee×·е\u0627]", status); gUnicodeSets[CWCF] = NEW_UNISET(u"[:CWCF:]", status); diff --git a/icu4c/source/test/intltest/numbertest_parse.cpp b/icu4c/source/test/intltest/numbertest_parse.cpp index 1fcaa1b0c93..4e091048d84 100644 --- a/icu4c/source/test/intltest/numbertest_parse.cpp +++ b/icu4c/source/test/intltest/numbertest_parse.cpp @@ -98,8 +98,9 @@ void NumberParserTest::testBasic() { for (auto cas : cases) { UnicodeString inputString(cas.inputString); UnicodeString patternString(cas.patternString); - const NumberParserImpl* parser = NumberParserImpl::createSimpleParser( - Locale("en"), patternString, parseFlags, status); + LocalPointer parser( + NumberParserImpl::createSimpleParser( + Locale("en"), patternString, parseFlags, status)); UnicodeString message = UnicodeString("Input <") + inputString + UnicodeString("> Parser ") + parser->toString(); @@ -111,9 +112,7 @@ void NumberParserTest::testBasic() { assertEquals( "Greedy Parse failed: " + message, cas.expectedCharsConsumed, resultObject.charEnd); assertEquals( - "Greedy Parse failed: " + message, - cas.expectedResultDouble, - resultObject.getDouble()); + "Greedy Parse failed: " + message, cas.expectedResultDouble, resultObject.getDouble()); } if (0 != (cas.flags & 0x02)) { @@ -133,17 +132,19 @@ void NumberParserTest::testBasic() { if (0 != (cas.flags & 0x04)) { // Test with strict separators - parser = NumberParserImpl::createSimpleParser( - Locale("en"), patternString, parseFlags | PARSE_FLAG_STRICT_GROUPING_SIZE, status); + parser.adoptInstead( + NumberParserImpl::createSimpleParser( + Locale("en"), + patternString, + parseFlags | PARSE_FLAG_STRICT_GROUPING_SIZE, + status)); ParsedNumber resultObject; parser->parse(inputString, true, resultObject, status); assertTrue("Strict Parse failed: " + message, resultObject.success()); assertEquals( "Strict Parse failed: " + message, cas.expectedCharsConsumed, resultObject.charEnd); assertEquals( - "Strict Parse failed: " + message, - cas.expectedResultDouble, - resultObject.getDouble()); + "Strict Parse failed: " + message, cas.expectedResultDouble, resultObject.getDouble()); } } }