You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
_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.
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.
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
)?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:
I will send in a CL if that looks okay.
The text was updated successfully, but these errors were encountered: