From: Andy Heninger Date: Mon, 31 Jul 2017 20:20:37 +0000 (+0000) Subject: ICU-12519 Break Iterator assignment handles Locales. X-Git-Tag: release-60-rc~207 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2b5557fce681e95378f747db75befb2fe15891db;p=icu ICU-12519 Break Iterator assignment handles Locales. X-SVN-Rev: 40301 --- diff --git a/icu4c/source/common/brkiter.cpp b/icu4c/source/common/brkiter.cpp index f8e4d984636..14a67095db1 100644 --- a/icu4c/source/common/brkiter.cpp +++ b/icu4c/source/common/brkiter.cpp @@ -195,7 +195,7 @@ BreakIterator::getAvailableLocales(int32_t& count) // ------------------------------------------ // -// Default constructor and destructor +// Constructors, destructor and assignment operator // //------------------------------------------- @@ -204,6 +204,19 @@ BreakIterator::BreakIterator() *validLocale = *actualLocale = 0; } +BreakIterator::BreakIterator(const BreakIterator &other) : UObject(other) { + uprv_strncpy(actualLocale, other.actualLocale, sizeof(actualLocale)); + uprv_strncpy(validLocale, other.validLocale, sizeof(validLocale)); +} + +BreakIterator &BreakIterator::operator =(const BreakIterator &other) { + if (this != &other) { + uprv_strncpy(actualLocale, other.actualLocale, sizeof(actualLocale)); + uprv_strncpy(validLocale, other.validLocale, sizeof(validLocale)); + } + return *this; +} + BreakIterator::~BreakIterator() { } diff --git a/icu4c/source/common/rbbi.cpp b/icu4c/source/common/rbbi.cpp index d5458e7e7e9..af18e7ac9e6 100644 --- a/icu4c/source/common/rbbi.cpp +++ b/icu4c/source/common/rbbi.cpp @@ -213,6 +213,8 @@ RuleBasedBreakIterator::operator=(const RuleBasedBreakIterator& that) { if (this == &that) { return *this; } + BreakIterator::operator=(that); + reset(); // Delete break cache information fBreakType = that.fBreakType; if (fLanguageBreakEngines != NULL) { @@ -311,16 +313,19 @@ RuleBasedBreakIterator::operator==(const BreakIterator& that) const { return FALSE; } + // The base class BreakIterator carries no state that participates in equality, + // and does not implement an equality function that would otherwise be + // checked at this point. + const RuleBasedBreakIterator& that2 = (const RuleBasedBreakIterator&) that; if (!utext_equals(fText, that2.fText)) { // The two break iterators are operating on different text, - // or have a different interation position. + // or have a different iteration position. + // Note that fText's position is always the same as the break iterator's position. return FALSE; }; - // TODO: need a check for when in a dictionary region at different offsets. - if (that2.fData == fData || (fData != NULL && that2.fData != NULL && *that2.fData == *fData)) { // The two break iterators are using the same rules. diff --git a/icu4c/source/common/unicode/brkiter.h b/icu4c/source/common/unicode/brkiter.h index b1e4cc68c6d..9c1ac7531bc 100644 --- a/icu4c/source/common/unicode/brkiter.h +++ b/icu4c/source/common/unicode/brkiter.h @@ -250,7 +250,7 @@ public: virtual int32_t next(void) = 0; /** - * Return character index of the current interator position within the text. + * Return character index of the current iterator position within the text. * @return The boundary most recently returned. * @stable ICU 2.0 */ @@ -277,7 +277,7 @@ public: virtual int32_t preceding(int32_t offset) = 0; /** - * Return true if the specfied position is a boundary position. + * Return true if the specified position is a boundary position. * As a side effect, the current position of the iterator is set * to the first boundary position at or following the specified offset. * @param offset the offset to check. @@ -331,7 +331,7 @@ public: * @param fillInVec an array to be filled in with the status values. * @param capacity the length of the supplied vector. A length of zero causes * the function to return the number of status values, in the - * normal way, without attemtping to store any values. + * normal way, without attempting to store any values. * @param status receives error codes. * @return The number of rule status values from rules that determined * the most recent boundary returned by the break iterator. @@ -469,7 +469,7 @@ public: static const Locale* U_EXPORT2 getAvailableLocales(int32_t& count); /** - * Get name of the object for the desired Locale, in the desired langauge. + * Get name of the object for the desired Locale, in the desired language. * @param objectLocale must be from getAvailableLocales. * @param displayLocale specifies the desired locale for output. * @param name the fill-in parameter of the return value @@ -482,7 +482,7 @@ public: UnicodeString& name); /** - * Get name of the object for the desired Locale, in the langauge of the + * Get name of the object for the desired Locale, in the language of the * default locale. * @param objectLocale must be from getMatchingLocales * @param name the fill-in parameter of the return value @@ -629,10 +629,12 @@ protected: /** @internal */ BreakIterator(); /** @internal */ - BreakIterator (const BreakIterator &other) : UObject(other) {} + BreakIterator (const BreakIterator &other); #ifndef U_HIDE_INTERNAL_API /** @internal */ - BreakIterator (const Locale& valid, const Locale& actual); + BreakIterator (const Locale& valid, const Locale &actual); + /** @internal. Assignment Operator, used by RuleBasedBreakIterator. */ + BreakIterator &operator = (const BreakIterator &other); #endif /* U_HIDE_INTERNAL_API */ private: @@ -640,12 +642,6 @@ private: /** @internal */ char actualLocale[ULOC_FULLNAME_CAPACITY]; char validLocale[ULOC_FULLNAME_CAPACITY]; - - /** - * The assignment operator has no real implementation. - * It's provided to make the compiler happy. Do not call. - */ - BreakIterator& operator=(const BreakIterator&); }; #ifndef U_HIDE_DEPRECATED_API @@ -661,5 +657,5 @@ U_NAMESPACE_END #endif /* #if !UCONFIG_NO_BREAK_ITERATION */ -#endif // _BRKITER +#endif // BRKITER_H //eof diff --git a/icu4c/source/test/intltest/rbbitst.cpp b/icu4c/source/test/intltest/rbbitst.cpp index d101c15aa46..7ffcae7222e 100644 --- a/icu4c/source/test/intltest/rbbitst.cpp +++ b/icu4c/source/test/intltest/rbbitst.cpp @@ -53,7 +53,6 @@ #define TEST_ASSERT_SUCCESS(errcode) { if (U_FAILURE(errcode)) { \ errcheckln(errcode, "Failure in file %s, line %d, status = \"%s\"", __FILE__, __LINE__, u_errorName(errcode));}} - //--------------------------------------------- // runIndexedTest //--------------------------------------------- @@ -107,6 +106,7 @@ void RBBITest::runIndexedTest( int32_t index, UBool exec, const char* &name, cha TESTCASE_AUTO(TestBug12918); TESTCASE_AUTO(TestBug12932); TESTCASE_AUTO(TestEmoji); + TESTCASE_AUTO(TestBug12519); TESTCASE_AUTO_END; } @@ -4792,6 +4792,40 @@ void RBBITest::TestEmoji() { } +// TestBug12519 - Correct handling of Locales by assignment / copy / clone + +// WHERE Macro yields a literal string of the form "source_file_name:line number " +// TODO: propose something equivalent as a test framework addition. + +#define WHERE __FILE__ ":" XLINE(__LINE__) " " +#define XLINE(s) LINE(s) +#define LINE(s) #s + +void RBBITest::TestBug12519() { + UErrorCode status = U_ZERO_ERROR; + LocalPointer biEn((RuleBasedBreakIterator *)BreakIterator::createWordInstance(Locale::getEnglish(), status)); + LocalPointer biFr((RuleBasedBreakIterator *)BreakIterator::createWordInstance(Locale::getFrance(), status)); + assertSuccess(WHERE, status); + assertTrue(WHERE, Locale::getEnglish() == biEn->getLocale(ULOC_VALID_LOCALE, status)); + assertTrue(WHERE, Locale::getFrench() == biFr->getLocale(ULOC_VALID_LOCALE, status)); + assertTrue(WHERE "Locales do not participate in BreakIterator equality.", *biEn == *biFr); + + LocalPointercloneEn((RuleBasedBreakIterator *)biEn->clone()); + assertTrue(WHERE, *biEn == *cloneEn); + assertTrue(WHERE, Locale::getEnglish() == cloneEn->getLocale(ULOC_VALID_LOCALE, status)); + + LocalPointercloneFr((RuleBasedBreakIterator *)biFr->clone()); + assertTrue(WHERE, *biFr == *cloneFr); + assertTrue(WHERE, Locale::getFrench() == cloneFr->getLocale(ULOC_VALID_LOCALE, status)); + + LocalPointerbiDe((RuleBasedBreakIterator *)BreakIterator::createLineInstance(Locale::getGerman(), status)); + UnicodeString text("Hallo Welt"); + biDe->setText(text); + assertTrue(WHERE "before assignment of \"biDe = biFr\", they should be different, but are equal.", *biFr != *biDe); + *biDe = *biFr; + assertTrue(WHERE "after assignment of \"biDe = biFr\", they should be equal, but are not.", *biFr == *biDe); +} + // // TestDebug - A place-holder test for debugging purposes. // For putting in fragments of other tests that can be invoked diff --git a/icu4c/source/test/intltest/rbbitst.h b/icu4c/source/test/intltest/rbbitst.h index 6809376647e..f5f6dfa6f76 100644 --- a/icu4c/source/test/intltest/rbbitst.h +++ b/icu4c/source/test/intltest/rbbitst.h @@ -79,6 +79,7 @@ public: void TestBug12918(); void TestBug12932(); void TestEmoji(); + void TestBug12519(); void TestDebug(); void TestProperties(); diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/rbbi/RBBITest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/rbbi/RBBITest.java index 07259a5c4bf..7f05b4b1329 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/rbbi/RBBITest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/rbbi/RBBITest.java @@ -957,4 +957,20 @@ public class RBBITest extends TestFmwk { } } + @Test + public void TestBug12519() { + RuleBasedBreakIterator biEn = (RuleBasedBreakIterator)BreakIterator.getWordInstance(ULocale.ENGLISH); + RuleBasedBreakIterator biFr = (RuleBasedBreakIterator)BreakIterator.getWordInstance(ULocale.FRANCE); + assertEquals("", ULocale.ENGLISH, biEn.getLocale(ULocale.VALID_LOCALE)); + assertEquals("", ULocale.FRENCH, biFr.getLocale(ULocale.VALID_LOCALE)); + assertEquals("Locales do not participate in BreakIterator equality.", biEn, biFr); + + RuleBasedBreakIterator cloneEn = (RuleBasedBreakIterator)biEn.clone(); + assertEquals("", biEn, cloneEn); + assertEquals("", ULocale.ENGLISH, cloneEn.getLocale(ULocale.VALID_LOCALE)); + + RuleBasedBreakIterator cloneFr = (RuleBasedBreakIterator)biFr.clone(); + assertEquals("", biFr, cloneFr); + assertEquals("", ULocale.FRENCH, cloneFr.getLocale(ULocale.VALID_LOCALE)); + } }