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

Jump to bottom

math: package refactoring #28490

Closed
petar-dambovaliev opened this issue Oct 30, 2018 · 6 comments
Closed

math: package refactoring #28490

petar-dambovaliev opened this issue Oct 30, 2018 · 6 comments
Labels
FrozenDueToAge

Comments

@petar-dambovaliev
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go1.11.1 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/petar/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/petar/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build612379143=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Browse the code
If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

What did you expect to see?

Code

What did you see instead?

There are some old files (2010-2011) in the math package that could use some refactoring,
Unnecessary variable allocations, unused named returns and so on.
I just wanted to ask if something like that would be accepted before i start doing work on it.
Thanks.
@adamdecaf
Copy link
Contributor

Can you provide specific examples or benchmarked improvements?

None of the public API can change in Go 1 and refactors need to be done such that no regressions occur.

@petar-dambovaliev
Copy link
Contributor Author

An example would be
https://github.com/golang/go/blob/master/src/math/big/int.go#L49

turns into https://play.golang.org/p/CDEyGeJrsf3

@ianlancetaylor ianlancetaylor changed the title Math package refactoring math: package refactoring Oct 30, 2018
@ianlancetaylor
Copy link
Contributor

Thanks, I don't think a change like that would be particularly helpful. At least in my opinion it's not significantly clearer than the current code. In general we try to avoid tweaking existing code unless the result is clearly better.

@petar-dambovaliev
Copy link
Contributor Author

Makes sense, thanks for the feedback.

@agnivade
Copy link
Contributor

Is there anything else to be done here ? Please close if not. Thanks.

@katiehockman
Copy link
Contributor

I'm going to close this for now.
@petar-dambovaliev please re-open if you still believe this is an issue. For any questions, feel free to use a forum, where there will be more people who can help you. See https://golang.org/wiki/Questions. Thanks.

@golang golang locked and limited conversation to collaborators Oct 31, 2019
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
6 participants
@adamdecaf @agnivade @ianlancetaylor @gopherbot @petar-dambovaliev @katiehockman