Issue 339290043: Adding currency names matcher to C++ number parsing code.

Can't Edit
Can't Publish+Mail
Start Review

Created:
6 years, 2 months ago by sffc

Modified:
6 years, 2 months ago

Reviewers:
andy.heninger

Base URL:
svn+icussh://source.icu-project.org/repos/icu/branches/shane/numberformat4/

Visibility:
Public.

More Reviews

Description

Adding currency names matcher to C++ number parsing code.

Patch Set 1 #

Total comments: 8

Created: 6 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+257 lines, -40 lines)			Patch
M	icu4c/source/common/ucurr.cpp	View	13 chunks	+94 lines, -31 lines	2 comments	Download
M	icu4c/source/common/ucurrimp.h	View	3 chunks	+15 lines, -0 lines	2 comments	Download
M	icu4c/source/i18n/Makefile.in	View	1 chunk	+2 lines, -1 line	0 comments	Download
M	icu4c/source/i18n/decimfmt.cpp	View	1 chunk	+2 lines, -1 line	0 comments	Download
A	icu4c/source/i18n/numparse_currency.h	View	1 chunk	+49 lines, -0 lines	2 comments	Download
A	icu4c/source/i18n/numparse_currency.cpp	View	1 chunk	+70 lines, -0 lines	0 comments	Download
M	icu4c/source/i18n/numparse_impl.h	View	2 chunks	+4 lines, -0 lines	2 comments	Download
M	icu4c/source/i18n/numparse_impl.cpp	View	2 chunks	+16 lines, -2 lines	0 comments	Download
M	icu4c/source/i18n/numparse_parsednumber.cpp	View	1 chunk	+1 line, -1 line	0 comments	Download
M	icu4c/source/i18n/numparse_types.h	View	1 chunk	+1 line, -1 line	0 comments	Download
M	icu4c/source/test/intltest/numbertest_parse.cpp	View	2 chunks	+3 lines, -3 lines	0 comments	Download

Messages

Total messages: 3

Expand All Messages | Collapse All Messages

sffc

Hi Andy, I'd like you to review this chunk of my parsing code. This is ...

6 years, 2 months ago (2018-02-09 10:59:38 UTC) #1

andy.heninger

https://codereview.appspot.com/339290043/diff/1/icu4c/source/common/ucurr.cpp File icu4c/source/common/ucurr.cpp (right): https://codereview.appspot.com/339290043/diff/1/icu4c/source/common/ucurr.cpp#newcode1307 icu4c/source/common/ucurr.cpp:1307: // Check for partial matches. Will this match half ...

6 years, 2 months ago (2018-02-10 01:26:44 UTC) #2

sffc

6 years, 2 months ago (2018-02-10 03:00:05 UTC) #3

Committed as revision 40889.

https://codereview.appspot.com/339290043/diff/1/icu4c/source/common/ucurr.cpp
File icu4c/source/common/ucurr.cpp (right):

https://codereview.appspot.com/339290043/diff/1/icu4c/source/common/ucurr.cpp...
icu4c/source/common/ucurr.cpp:1307: // Check for partial matches.
On 2018/02/10 01:26:44, andy.heninger wrote:
> Will this match half of a supplemental character?

Hmm, yes, it would, but I don't see how it would be an issue since strings
aren't accepted unless there is a complete match.

https://codereview.appspot.com/339290043/diff/1/icu4c/source/common/ucurrimp.h
File icu4c/source/common/ucurrimp.h (right):

https://codereview.appspot.com/339290043/diff/1/icu4c/source/common/ucurrimp....
icu4c/source/common/ucurrimp.h:48: * this may be nonzero even if no full
currency was matched.
On 2018/02/10 01:26:44, andy.heninger wrote:
> Is this allowed to be null?

No.  Added to the docstring.

https://codereview.appspot.com/339290043/diff/1/icu4c/source/i18n/numparse_cu...
File icu4c/source/i18n/numparse_currency.h (right):

https://codereview.appspot.com/339290043/diff/1/icu4c/source/i18n/numparse_cu...
icu4c/source/i18n/numparse_currency.h:26: CurrencyNamesMatcher() = default;  //
WARNING: Leaves the object in an unusable state
On 2018/02/10 01:26:44, andy.heninger wrote:
> Needs to leave it in good enough shape for the destructor to run safely.

The CharString is default-initialized also, so it should be okay.

https://codereview.appspot.com/339290043/diff/1/icu4c/source/i18n/numparse_im...
File icu4c/source/i18n/numparse_impl.h (right):

https://codereview.appspot.com/339290043/diff/1/icu4c/source/i18n/numparse_im...
icu4c/source/i18n/numparse_impl.h:47: // WARNING: All of these matchers start in
an uninitialized state.
On 2018/02/10 01:26:44, andy.heninger wrote:
> Default initialized is different from uninitialized. Some constructor always
> runs, and needs to get everything in sufficiently good shape that destructors
> are safe. Uninitialized sounds like naked raw memory, which it is not.

Done.

Expand All Messages | Collapse All Messages