[go-nuts] in the func unicode/utf16 EncodeRune can remove this code?

On Sun, Dec 15, 2013 at 10:34 PM, 产学红 wrote:
hi,all:

in unicode/utf16 EncodeRune:

func EncodeRune(r rune) (r1, r2 rune) {

if r < surrSelf || r > maxRune || IsSurrogate(r) {

return replacementChar, replacementChar

}

r -= surrSelf

return surr1 + (r>>10)&0x3ff, surr2 + r&0x3ff

}


can remove || IsSurrogate(r) ???

--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Dec 15, 2013 6:34 AM, "产学红" wrote:
hi,all:

in unicode/utf16 EncodeRune:
func EncodeRune(r rune) (r1, r2 rune) {
if r < surrSelf || r > maxRune || IsSurrogate(r) {

return replacementChar, replacementChar

}

r -= surrSelf

return surr1 + (r>>10)&0x3ff, surr2 + r&0x3ff

}
can remove || IsSurrogate(r) ???

On Mon, Dec 16, 2013 at 4:06 AM, minux wrote:
it could be removed because the first condition contains it. as i said in
the reply to your another related topic, please send a CL.

--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

On Sun, Dec 15, 2013 at 5:09 PM, Dave Cheney wrote:

If you do remove IsSurrogate from that check then there is 0 coverage
of IsSurrogate.

If you want to propose a CL to make this change, please ensure that
you add additional testing of IsSurrogate.

On Mon, Dec 16, 2013 at 9:14 AM, minux wrote:
as the first condition (r < surrSelf) strictly includes IsSurrogate(r), I
don't think
that qualifies as test coverage of IsSurrogate.

Anyway, I think we do need to test IsSurrogate if it deemed not simple
enough
to be not tested.