On 10/25/2012 06:22 AM, Liviu Nicoara wrote:
On 10/24/12 11:11, Martin Sebor wrote:
On 10/24/2012 07:12 AM, Liviu Nicoara wrote:
[...]
I modified the test according to the suggestions. The test fails all
corresponding wide-char cases and I am investigating other potential
defects as well. For example, I do not think that employing strcoll and
wcscoll in compare is correct as they stop at the first NUL, although
strings may contain characters after the NUL that alter the result of
the comparison.

I would expect the wchar_t specialization to be analogous
to the narrow one. In fact (without looking at the code),
I would even think both could be implemented in terms of
the same function template specialized on the character
type and on the libc string function. (Although I'm not
necessarily suggesting this as the solution to this issue.)


They are not similar, AFAICT. In revision 367462 you implemented the
wide byname do_compare in terms of wcscoll, if available, when using
libc. That implementation does not take into account embedded NULs. This
is in contrast with the narrow byname which simply transforms the
strings and compares them.

IIUC, rev 367462 actually implements the function in terms of
do_transform() when wcscoll() isn't available as a workaround.
Ironically, the workaround is actually better than the default
implementation.


Testing shows that an implementation of wide byname do_compare identical
with the narrow version passes all tests, as expected.

It's easy to me to just remove the wcscoll-based implementation. Also,
error reporting from wcscoll seems difficult to use when libc does not
have thread-local errno's, and right now we don't check for errors.
OTOH, I expect wcscoll to be faster than the (simpler) transformation
followed by comparison.

I incline towards the simpler approach. Thoughts?

There are comments suggesting that calling do_transform()
on the whole string may be suboptimal. Intuitively it makes
sense that calling wcscoll() (in a loop, on the NUL-terminated
substrings, if necessary) should be faster than simply calling
do_transform() followed by wstring::compare(), but it would
make sense to confirm the hypothesis before implementing the
optimization.

Martin


Liviu

Reply via email to