perf: skip joining the int64 parsers' errors for an empty tag (#107)

fillField offers an int64-kinded field's tag to time.ParseDuration and
then to strconv.ParseInt, and since #96, when both reject it, returns
parseErr(fmt.Errorf("%w; %w", err, intErr)). Both reject an empty tag,
and parseErr returns nil for one, so every Set of a zero time.Duration
or other int64-kinded field tagged `default:""` formatted both
messages into a joined error and threw it away. make bench-compare
BASE=v1.10.0 put BenchmarkParse/empty_tag at 143.4 -> 470.1 ns (+228%,
p=0.002) and 80 B / 2 allocs -> 352 B / 10. #96 is not released yet.

The last branch of the int64 case is now `else if defaultVal != ""`,
so the errors are joined only for a tag parseErr reports. An empty tag
falls out of the switch, and fillField returns (false, nil), as
parseErr's nil made it do before. Both parsers still run on it, as the
bool, int, uint and float parsers do, which is v1.10.0's cost. The
check is on the tag as written, so a blank tag, which the duration
attempt trims to nothing, still reports both rejections. The call
stays in parseErr, which reports a type's own unmarshaler rejection in
place of the joined errors (#90), and no unmarshaler is offered an
empty tag, so the check never skips a rejection. The comment on the
int64 case says why the check is there, and BenchmarkParse/empty_tag's
comment no longer says the errors are joined.

Not a behavior change: nothing that succeeded fails, nothing that
failed succeeds, and no message changes. Master's suite passes with
four wrong versions of the check too, so three pins are added for the
int64 branch. Each passes on master's set.go:

- TestSet_DurationEmptyTagReportsNothing: `default:""` on a
  time.Duration and an int64 is no error and leaves both zero. Nothing
  in make test put an empty tag on an int64-kinded field; only
  BenchmarkParse/empty_tag's check did, under make bench-smoke.
- TestSet_WhitespaceTagIsNotEmpty/an_int64_fails_to_parse_it: " " on
  an int64 reports both rejections, the duration parser's for "".
- TestSet_FailingUnmarshalerErrorIsReported gains a row for
  umFailingDuration, an int64-kinded type whose UnmarshalText always
  fails: "1d" reports that rejection alone. No test gave an
  int64-kinded type a failing unmarshaler.

go test -race -shuffle=on -v ./... gives 322 PASS lines and no
failures, against 319 on master, and master's set.go with the new
tests gives 322 too. make cover gives 100.0%, make bench-smoke passes
all 45 cases, and gofmt -s -l and go vet are clean. A throwaway probe,
not committed, logs the same on master and on this commit:
`default:""` on a field of every kind, int64-kinded types with failing
unmarshalers among them, is no error and leaves the same values; "1d",
"abc", " ", "\t" and numbers past either end of int64's range give the
same messages, errors.Is matches strconv.ErrSyntax or strconv.ErrRange
as before, and errors.As reaches the *strconv.NumError; and on an
int64-kinded type, a failing unmarshaler's rejection is reported
alone.

Mutation-checked, 5 of 7 killed; master's tests kill 1 of them.
Inverting the check fails 11 tests and subtests, 7 of them on master's
tests. Checking the trimmed tag fails the blank int64 subtest.
Building the error without parseErr fails the int64 unmarshaler row,
and with the check removed as well, the empty-tag test too. Skipping
the error when an unmarshaler rejected the tag fails the row. Two
survive, both the same as this commit in behavior: master's plain
else, which differs in cost only, and adding `|| unmarshalErr != nil`,
since unmarshalErr is always nil for an empty tag.

make bench-compare BASE=origin/master
BENCH='Parse/(empty_tag|int64|duration|kinds)', 6 interleaved rounds
at 400ms, Go 1.26.5 darwin/arm64:

    Parse/empty_tag  455.9 ns -> 148.3 ns   -67.46% (p=0.002)
                     352 B / 10 allocs -> 80 B / 2
    Parse/int64      179.2 ns, 181.0 ns     ~ (p=0.188), 48 B / 3 both
    Parse/duration   157.3 ns, 159.4 ns     ~ (p=0.058), 16 B / 2 both
    Parse/kinds      2.058 µs, 2.051 µs     ~ (p=0.457), 128 B / 17 both

The last three give the int64 branch a tag one of its parsers takes,
so none reaches the changed line. Against v1.10.0, with
BENCH='Parse/(unparsed_kind|empty_tag)', empty_tag is 80 B / 2 allocs
on both sides, at 137.0 and 149.0 ns, and unparsed_kind, the cost
every row includes, is 86.74 and 93.93 ns, so most of the time left
over comes from outside the int64 branch.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: creasty

benchmark_test.go

@@ -187,7 +187,7 @@ func BenchmarkParse(b *testing.B) {
 		})
 	})
 
-	// Both int64 parsers fail and their errors are joined before the empty tag is let through.
+	// Both int64 parsers fail on the empty tag, which is let through without joining their errors.
 	b.Run("empty_tag", func(b *testing.B) {
 		type st struct {
 			V time.Duration `default:""`

set.go

@@ -250,11 +250,14 @@ func fillField(field reflect.Value, tag fieldTag, isInitial bool, pending *pendi
 			// The duration attempt tolerates surrounding whitespace, the numeric fallback does not.
 			// When both fail, both errors are reported, since the tag may have been meant for either,
 			// unless the type's own unmarshaler rejected it first: parseErr reports that rejection.
+			// An empty tag fails both as well, and parseErr would report nothing for it, so the
+			// errors are joined only for a tag that is not empty: the join formats both messages,
+			// which every empty tag would otherwise pay for.
 			if val, err := time.ParseDuration(strings.TrimSpace(defaultVal)); err == nil {
 				field.Set(reflect.ValueOf(val).Convert(field.Type()))
 			} else if val, intErr := strconv.ParseInt(defaultVal, 0, 64); intErr == nil {
 				field.Set(reflect.ValueOf(val).Convert(field.Type()))
-			} else {
+			} else if defaultVal != "" {
 				return false, parseErr(fmt.Errorf("%w; %w", err, intErr))
 			}
 		case reflect.Uint:

set_duration_test.go

@@ -73,6 +73,22 @@ func TestSet_DurationErrorNamesBothParsers(t *testing.T) {
 	assert.ErrorAs(t, err, &numErr, "the numeric parser's error stays reachable")
 }
 
+// TestSet_DurationEmptyTagReportsNothing covers `default:""` on an int64-kinded field. Both parsers
+// reject an empty tag, as they reject "1d", but it asks for the zero value rather than for anything
+// to be parsed, so Set reports neither rejection and leaves the field zero. The int64 branch checks
+// for an empty tag before joining the two errors, so the int in TestSet_EmptyTag does not cover it.
+func TestSet_DurationEmptyTagReportsNothing(t *testing.T) {
+	type sample struct {
+		Duration time.Duration `default:""`
+		Int64    int64         `default:""`
+	}
+
+	var got sample
+	require.NoError(t, defaults.Set(&got))
+
+	assert.Zero(t, got)
+}
+
 // TestSet_DurationStringOnANarrowerIntegerIsRejected pins that the duration fallback is int64-only.
 // A narrower width rejects a duration string outright, where it used to keep its zero value and
 // report success.

set_error_test.go

@@ -278,7 +278,9 @@ func TestSet_WellFormedJSONOfTheWrongShape(t *testing.T) {
 }
 
 // TestSet_WhitespaceTagIsNotEmpty pins that a stray space is a value rather than an absence: a
-// string takes it verbatim, and every container tries to decode it as JSON and fails.
+// string takes it verbatim, and every container tries to decode it as JSON and fails. An int64
+// fails both of its parsers: the duration attempt trims the space to nothing, but the tag is not
+// empty, so both rejections are reported.
 func TestSet_WhitespaceTagIsNotEmpty(t *testing.T) {
 	t.Run("a string takes it", func(t *testing.T) {
 		got := struct {
@@ -290,6 +292,15 @@ func TestSet_WhitespaceTagIsNotEmpty(t *testing.T) {
 		assert.Equal(t, " ", got.S)
 	})
 
+	t.Run("an int64 fails to parse it", func(t *testing.T) {
+		got := struct {
+			V int64 `default:" "`
+		}{}
+
+		assert.EqualError(t, defaults.Set(&got),
+			`field V: invalid default " ": time: invalid duration ""; strconv.ParseInt: parsing " ": invalid syntax`)
+	})
+
 	tests := []struct {
 		name string
 		ptr  interface{}

set_unmarshaler_test.go

@@ -136,6 +136,14 @@ func (u *umFailingBoth) UnmarshalJSON([]byte) error {
 	return errors.New("json fails")
 }
 
+// umFailingDuration always fails, and is int64-kinded, so parsing by kind tries both int64 parsers,
+// whose errors are joined when both reject the tag too.
+type umFailingDuration time.Duration
+
+func (u *umFailingDuration) UnmarshalText([]byte) error {
+	return errors.New("always fails")
+}
+
 // umDuration wraps a duration to give it a text format, which makes it struct-kinded: parsing by
 // kind hands its tag to encoding/json.
 type umDuration struct {
@@ -388,7 +396,8 @@ func TestSet_FailingUnmarshalerFallsBackToKind(t *testing.T) {
 // TestSet_FailingUnmarshalerErrorIsReported covers a tag the type's own unmarshaler rejects and
 // parsing by kind cannot take either. The error Set returns names that rejection as its cause.
 // For a type that implements both interfaces, the tag goes to UnmarshalText first, so its
-// rejection is the one named.
+// rejection is the one named. An int64-kinded type is listed too: both of its parsers fail as well,
+// and the rejection is named rather than their joined errors.
 //
 // The cause used to be the failed parse by kind, because the rejection was discarded. For a
 // struct-kinded type, that was a syntax error from encoding/json, a parser the tag was never
@@ -403,6 +412,9 @@ func TestSet_FailingUnmarshalerErrorIsReported(t *testing.T) {
 	type both struct {
 		Level umFailingBoth `default:"x"`
 	}
+	type int64Kind struct {
+		Delay umFailingDuration `default:"1d"`
+	}
 
 	tests := []struct {
 		name string
@@ -412,6 +424,7 @@ func TestSet_FailingUnmarshalerErrorIsReported(t *testing.T) {
 		{"text", &textOnly{}, `field Timeout: invalid default "garbage": time: invalid duration "garbage"`},
 		{"JSON", &jsonOnly{}, `field Level: invalid default "x": always fails`},
 		{"both", &both{}, `field Level: invalid default "x": text fails`},
+		{"text, on an int64 kind", &int64Kind{}, `field Delay: invalid default "1d": always fails`},
 	}
 
 	for _, tt := range tests {