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

runtime: unusual code in proc.go could use documentation #20717

Closed
velovix opened this issue Jun 18, 2017 · 4 comments
Closed

runtime: unusual code in proc.go could use documentation #20717

velovix opened this issue Jun 18, 2017 · 4 comments
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge

Comments

@velovix
Copy link

velovix commented Jun 18, 2017

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

go version devel +d293c85e39 Sat Jun 17 08:17:14 2017 +0000 linux/amd64

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

GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/velovix/go"
GORACE=""
GOROOT="/home/velovix/go-source"
GOTOOLDIR="/home/velovix/go-source/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build407292918=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

Please see this discussion on golang-nuts:
https://groups.google.com/d/topic/golang-nuts/FnF3oZeJ7aY/discussion

There is some code at lines 198-202 in /src/runtime/proc.go that could do with a bit of documentation. The code in question can be found here on Github.

Here is my current draft:

// In case exit fails to stop the program, try to stop it by causing a
// panic. If that doesn't work, stall the program.
// This is here to help catch problems with new ports, where exit may not
// be properly implemented.

I will send in a CL if that looks okay.
@gopherbot gopherbot added the Documentation Issues describing a change to documentation. label Jun 18, 2017
@ALTree ALTree changed the title Unusual code in proc.go could use documentation runtime: unusual code in proc.go could use documentation Jun 18, 2017
@ALTree
Copy link
Member

ALTree commented Jun 18, 2017

I will send in a CL if that looks okay.

Just sending the CL is fine: it will be reviewed (and commented on) on gerrit. Remember to link this issue in the commit message.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/46212 mentions this issue.

@robpike
Copy link
Contributor

robpike commented Jun 21, 2017 •
edited
Loading

Comment I added to CL:

_I've changed my mind. This doesn't need careful documentation. It's a common pattern for code in runtimes - and other systems - to cause deliberate crashes when things are clearly not working.

To someone reading the code, it should be very clear that this code will cause a trap, and if the reader is not accustomed to reading runtimes, a glance to the line immediately preceding should make it very clear why we're trapping._

If you follow this to its conclusion, you'll end up needing to comment everything about how everything works for people unfamiliar with such systems. That's not a good precedent.

@velovix
Copy link
Author

velovix commented Jun 22, 2017

I did not realize that this snippet existed more than once in the runtime. If it's a pattern, I can see why we wouldn't want a long comment block explaining it.

@velovix velovix closed this as completed Jun 22, 2017
@golang golang locked and limited conversation to collaborators Jun 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@velovix @ALTree @robpike @gopherbot