Skip to content
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

unicode/utf16: redundant code in EncodeRune #6956

Closed
gopherbot opened this issue Dec 15, 2013 · 4 comments
Closed

unicode/utf16: redundant code in EncodeRune #6956

gopherbot opened this issue Dec 15, 2013 · 4 comments

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 15, 2013

by chanxuehong:

in the func EncodeRune, it seems the code "|| IsSurrogate(r)" can be removed,
yes?

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
}
@minux
Copy link
Member

@minux minux commented Dec 15, 2013

Comment 1:

As dave suggested, please consider add separate test for IsSurrogate along with this
change.

Labels changed: added release-none, repo-main.

Status changed to Accepted.

@davecheney
Copy link
Contributor

@davecheney davecheney commented Dec 16, 2013

Comment 2:

This issue was updated by revision fb31a0b.

Add tests for IsSurrogate.
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/42570043

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Dec 16, 2013

Comment 3 by chanxuehong:

thks all, I found in the same func the "0x3ff" of "surr1 + (r>>10)&0x3ff" can be
removed. 
like this:
return surr1 + r>>10, surr2 + r&0x3ff

@griesemer
Copy link
Contributor

@griesemer griesemer commented Oct 1, 2014

Comment 4:

Re: #3: It's correct that one can remove &0x3ff . Leaving for symmetry (doesn't really
cost).
This was fixed with https://golang.org/cl/42570043 .

Status changed to Fixed.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants