THRIFT-5806: Count zero set fields on a nil union rather than dereferencing it

[slachiewicz]

JIRA: THRIFT-5806
Client: go

CountSetFields read the union's fields straight off the receiver, so a struct holding a nil union field panicked the moment it was serialized:

panic: runtime error: invalid memory address or nil pointer dereference

Returning 0 for a nil receiver lets the union's own arity check speak instead:

*Descendant error writing struct: *Descendant write union: exactly one field must be set (0 set)

What this deliberately does not do

An earlier revision of this PR also made the struct writer skip unset default-requiredness pointer fields, so the field would be omitted the way Python, Java, Node.js and C# omit it. That half is now THRIFT-6204, because it is a wire change and not a panic fix.

is_pointer_field() is true for every struct, union and exception field whatever its requiredness, and for every cpp.ref field, so the guard reaches much further than unions. Measured by regenerating every IDL under test/, lib/go/test/ and tutorial/ with and without it: 75 of 650 generated files change, 227 write sites gain a guard, and 59 of the changed files hold service Args/Result structs — so RPC argument and result encoding is in scope.

It also changes bytes for a case that never crashed. A nil field whose struct has no members is written today as an empty struct:

master:  0c 00 01 00 08 00 02 00 00 00 07 00    field 1 present, empty struct
guarded:       08 00 02 00 00 00 07 00          field 1 absent

and a peer declaring that field required accepts the first and rejects the second with Required field E is not set.

That is worth doing, but on its own ticket where the compatibility question gets a proper answer. This PR is now only the nil guard: no generated write path changes, no wire bytes change.

Tests

StructWithUnsetUnion in UnionDefaultValueTest.thrift plus TestNilUnion and TestStructWithUnsetUnion.

Verified: with the master generator TestNilUnion fails with panic: runtime error: invalid memory address or nil pointer dereference; with this change go test ./tests/ -count=1 in lib/go/test passes.

This change was created with AI assistance (Claude Opus 5).

[Jens-G]

Code review

Found 1 issue:

1. The commit has no AI attribution trailer, although the PR description says the change was created with AI assistance (AGENTS.md says "Always label AI-assisted commits and PRs. Use one or both of: Co-Authored-By: <AI tool name and version> / Generated-by: <AI tool name and version>"). The PR text names no tool or version either. The commits of Cap every build workflow job at 60 minutes #3816 and THRIFT-6200: Use the declaring package for enum arguments in the Go -remote stub #3817 carry such a trailer.

thrift/AGENTS.md

Lines 60 to 74 in 3eeb97b

	- **Always** label AI-assisted commits and PRs. Use one or both of:
	```
	Co-Authored-By: <AI tool name and version>
	Generated-by: <AI tool name and version>
	```
	Example:
	```
	THRIFT-9999: Fix connection timeout handling in Go client
	Client: go
	Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
	```
	- Apply this label even when AI only generated a portion of the change.
	- **Never** put tool-internal links into a commit message, PR description, JIRA comment or e-mail.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[slachiewicz]

Rebased on master and added the Co-Authored-By: Claude Opus 5 trailer to the commit; the PR description now names the tool as well. Patch content is unchanged (8517fd8).

This comment was created with AI assistance.