/cc some of the runtime folks @aclements @RLH.

The existing docs is fine as is.

"The argument obj must be a pointer to an object allocated by
calling new, by taking the address of a composite literal, or by
taking the address of a local variable."

This means no other pointers are accepted, which includes
package level variables.

We shouldn't document whether it checks the incoming obj
pointer or not, because that will limit future implementations.

@minux, "This means no other pointers are accepted, which includes package level variables."
This is not the fact. Package level variables are acceptable: https://play.golang.org/p/tH_3uwbgUw
The document is surely wrong.

If the document is intended to exclude package level variables, please also exclude package level pointers to objects allocated by calling new and by taking the address of composite literals.

But if the document doesn't forbid package level pointers to objects allocated by calling new and by taking the address of composite literals, why it forbids general package level pointers?

If the document has no problems, can I file an issue for the compiler implementation?
After all, older compiler versions will create a panic when argument obj is a pointer to package level data.

Whatever, the docs for SetFinalizer should be more detailed to address the differences between documented behavior and actual implemented behavior.

Good explanation, ok, I don't insist on changing the docs any more.
But I think putting a warning to using pointers to package level data in docs will be perfect.

There is a slight bug in the documentation. As written, it technically guarantees that SetFinalizer will panic if passed a pointer to a package-level variable. That clearly isn't the case. In fact, this claim is nonsense even if you ignore package-level variables, since we can't tell the difference between passing, say, a pointer to a composite literal and a pointer to its first element. So we should change this to say that it "may abort the program."

However, I'm inclined to change the guarantee here as well (and also the docs) to accept pointers to package-level variables just to simplify the API and its specification. I can't see how this would unduly constrain future implementations. @RLH?

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

@aclements @RLH Can we close this issue now?

@RLH and I chatted about whether to guarantee that SetFinalizer won't panic if passed a package-level variable. Neither of us have strong feelings about this either way, but the existing non-guarantee is more permissive for the implementation of finalizers and lets us change our minds later if there's more evidence. Further, it's important in general that SetFinalizer only be called for the beginning of an object, but we can't check that for package-level data, so we're leaning towards not trying to make any guarantees about pointers to package-level variables.

Hence, we're going to leave the documentation alone unless there's more compelling evidence for specifying this than the documentation being more restrictive than the implementation.

@aclements, @RLH,

the original title of this issue is "Make the SetFinalizer docs perfect".
I just feel the docs is some verbose.

For example, in the tip version, the docs says:

The argument obj must be a pointer to an object allocated by calling
new, by taking the address of a composite literal, or by taking the
address of a local variable.

So I think the following SetFinalizer callings for a, b, c will be always safe in later go versions:

package main

import (
    "runtime"
)

func callback1(*[]int) {}
func callback2(*int) {}

var a = &[]int{}
var b = new(int)

func main() {
    v := 123
    c := &v

    runtime.SetFinalizer(a, callback1)
    runtime.SetFinalizer(b, callback2)
    runtime.SetFinalizer(c, callback2)

    // ...

    // ....
}

However, the docs then says this:

It is not guaranteed that a finalizer will run for objects allocated
in initializers for package-level variables. Such objects may be
linker-allocated, not heap-allocated.

Now, I can't confirm if the SetFinalizer callings for a, b will be always safe or not.

@yaxinlx, maybe I don't understand what you mean by SetFinalizer being "safe". To me, SetFinalizer is "safe" in situations it guarantees won't panic. But your second quote from the documentation doesn't say anything about SetFinalizer panicking. It just says the finalizer may not run for a or b (which is true in general, since finalizers are never guaranteed to be run).

The complaint for the docs is I don't understand what the "must" mean in the first quote.
"must be xxx to possibly trigger a finalizer calling", "must be xxx to avoid paniking in later go versions"?

Thanks for your explanation. By your explanation, I think "must" means "must be xxx to avoid paniking in later go versions".

The complaint for the docs is I don't understand what the "must" mean in the first quote.
"must be xxx to possibly trigger a finalizer calling", "must be xxx to avoid paniking in later go versions"?

Either may be true. The point is that this is an intentionally underspecified contract between SetFinalizer and the caller of SetFinalizer. "The argument obj must be a pointer to an object allocated by calling new, by taking the address of a composite literal, or by taking the address of a local variable." means it's the caller's responsibility to pass an obj that satisfies these requirements. If the caller does something else, the behavior of SetFinalizer is unspecified. It may panic (a possibility suggested by the last sentence in that paragraph). It may do nothing (in which case the finalizer will never run). It may reboot your computer (though that would be rude). Who knows? And, as you point out, it may behave differently in other versions of Go or other implementations of Go (e.g., gccgo, GopherJS).