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.

Sign up for GitHub

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

Jump to bottom

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
Labels
FrozenDueToAge

Comments

@gopherbot
Copy link

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 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

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

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

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.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@davecheney @minux @griesemer @gopherbot