-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rich0003/font1635 update icu to 72 1 #20
base: runtimecore
Are you sure you want to change the base?
Conversation
Revise uses of UVector in Formatting related code to better handle memory allocation failures. This is one of an ongoing series of commits to address similar problems with UVector usage throughout ICU. The changes primarily involve switching uses of UVector::addElementX() to the new adoptElement() or addElement() functions, as appropriate, and using LocalPointers for tracking memory ownership.
…or vehicle-fuel) See unicode-org#1947
1. vector.contains() uses sequential comparison, O(n). As the vector size is great, the performance will be impacted. Remove this unnecessary check, vector.contains(), in C++. 2. At Java's CjkBreakEngine, replace "vector.contains()" with "if(pos > previous)" to deal with duplicate breakpoint position. This way, C++ and Java implementation will be synchronous. Test: ant checkTest -Dtestclass='com.ibm.icu.dev.test.rbbi.RBBITest' (RBBTest#TestBreakAllChars() can generate duplicate position for word break. It could pass with this change)
…pattern; includes new getter/setter API per TC discussion.
gcc 5.5 on Solaris refuses to recognise pow(int, int32_t)
Bumps xercesImpl from 2.12.0 to 2.12.2. --- updated-dependencies: - dependency-name: xerces:xercesImpl dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
Remove the functions UVector::addElementX() and UVector::ensureCapacityX() that were temporarily added to aid in the cleaning up of UVector's out-of-memory error handling.
1. \u30fc doesn't belong to Hira, Kana nor Han. Add it into CJK dictionary 2. Include fullwidth char into ALPlus
…n of adding performance tests to ICU CI. - test/perf/Makefile.in: adds strsrchperf to list of subdirs. changes target 'all' to compile everything in the standard way. - test/perf/ustrperf/Makefile.in: changes target executable from stringperf to ustrperf (i.e. name of directory) to allow uniform handling with other perf tests in GHA CI rules. - tools/ctestfw/uperf.cpp: changes output to ndjson format for processing with GHA Benchmark. Keep the previous output, which gets processed by the Perl scripts, when executed in 'verbose' mode. Backward compatibility, in case someone still wants to use the Perl scripts for the time being. May get cleaned up later. Also remove a few non-essential output lines that would interfer with GHA Benchmark. processing
ICUNotifier::notifyChanged() was using the thread-unsafe double-checked lock idiom. Replace it with use of the mutex only.
…fferent default locales.
After updating icu4c displayoptions.h
https://runtime-rtc.esri.com/view/vTest/job/vtest/job/3rdparty-interface/341/downstreambuildview/
|
@chri7325 Could you review when you get chance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I would suggest amending the commits to provide a bit more detail for data creation and the deprecated functions. These can probably be mostly copy pasted from the last time @robert-craig-houston created the data but if there are any changes, it helps tremendously to document that step in the history for the next upgrade.
@@ -323,7 +323,7 @@ ucnv_safeClone(const UConverter* cnv, void *stackBuffer, int32_t *pBufferSize, U | |||
U_CAPI UConverter* U_EXPORT2 | |||
ucnv_clone(const UConverter* cnv, UErrorCode *status) | |||
{ | |||
return ucnv_safeClone(cnv, nullptr, nullptr, status); | |||
return ucnv_clone(cnv, status); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were these changes needed to compile? I wonder if these need to be upstreamed? Then we can back this commit out and cleanly merge next upgrade.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes were needed for winuwp to compile. It seems to be more picky than the others.
When you say push them upstream, do you mean as a PR to the github/icu repository?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. It has been a while since I upstreamed a change to ICU but I remember the maintainers were very fast and receptive. If we don't upstream this, we'll probably need to make these changes every time we upgrade so removing that would benefit our process and make upgrades more trivial.
Thanks @chri7325 . For breaking up the commits, do you mean apply them as separate PRs so it's easier to distinguish:
|
Luckily no, that would be awful haha. I was just thinking that you can amend your current commits with the more context for the change and the commands you used to make them. You can easily amend commit messages with an interactive rebase and force push them here. They help a lot more with 3rdparty libraries since there's no other contacts we have for working on them to find out why a change was made. Here's the example I was talking about 68e8dc2 |
#issue: https://devtopia.esri.com/runtime/fontana/issues/1635
Update esri/icu/runtimecore to use icu 72.1