I had a test that called flag.NewFlagSet followed by FlagSet.SetOutput(io.Discard). Yet, in some cases, the flagset would print straight to stderr, ignoring my instructions.

I reduced the code into this reproducer: https://go.dev/play/p/KgDpqcSeR2c

When reduced, it's relatively straightforward to see what is going on in the non-pointer case:

1. The code calls NewFlagSet, which creates pointer P1, and sets f.Usage = f.defaultUsage with P1 as the method receiver parameter.
2. The code then dereferences NewFlagSet, resulting in a variable of non-pointer type FlagSet, making a shallow copy of the struct.
3. The code calls SetOutput, a method with a pointer receiver. It's equivalent to calling (&f2).SetOutput; note that this is a pointer to the stack, P2. P2's output field is updated, but not P1's.
4. When calling f2.Parse, we ultimately end up calling f2.Usage on P2, which is still to P1's defaultUsage method.
5. As such, we use P1's output field to print the usage, not P2's.

Arguably this is my mistake for dereferencing NewFlagSet, and I tend to agree. The buggy test was, in fact, in the standard library itself:

I think we should add a noCopy field to the FlagSet struct to make vet error when users make full copies of flagsets. In some cases that can be fine, such as when the user doesn't call SetOutput, but in general I think it's confusing and can easily lead to bugs. And, most importantly, I don't see why anyone would ever want to make a shallow copy of a FlagSet like this. If we wanted to support a full "clone" operation, it would have to be a new feature without the output field bug above.

Another option would be to remove the f.Usage = f.defaultUsage line from NewFlagSet, so that then P2's call to the usage method would go into if f.Usage == nil { f.defaultUsage }, calling P2's defaultUsage rather than P1's. However, I think this could be a backwards incompatible change, as f := NewFlagSet(); f.Usage() works today, and it would start panicking with such a change.

Yet another option is to document this pointer dereference gotcha in the FlagSet docs. Certainly better than nothing, but I reckon that a vet check is still a much better outcome.