From 6bf4cd11cf7c7471b3f205c041840e3642f3e8d2 Mon Sep 17 00:00:00 2001 From: youdie006 Date: Thu, 13 Aug 2026 10:14:47 +0900 Subject: [PATCH 1/2] size: fix compile error for a fixed-size convert shim A "msgp:shim T as:BASE mode:convert" directive made the size generator emit a Msgsize that declared a temporary for the converted base value and never assigned it. For a fixed-size base (int64, float64, ...) the size is a compile-time constant that ignores the temporary, so the generated code failed to compile with "declared and not used". For a fixed-size base, emit the constant size directly and skip the temporary. Variable-size bases are intentionally left as-is: computing their exact size would require calling the shim in Msgsize, which can be more expensive than the resulting under-allocation. Add a fixed-size (int64) convert shim fixture and a round-trip test. Fixes #446 --- _generated/convert.go | 19 +++++++++++++++++++ _generated/convert_test.go | 20 ++++++++++++++++++++ gen/size.go | 27 +++++++++++++++++---------- 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/_generated/convert.go b/_generated/convert.go index 964cff7d..7b9b4991 100644 --- a/_generated/convert.go +++ b/_generated/convert.go @@ -81,3 +81,22 @@ type ConvertErrVal string type ConvertErr struct { Err ConvertErrVal } + +//msgp:shim ConvertIntVal as:int64 using:fromConvertIntVal/toConvertIntVal mode:convert +//msgp:ignore ConvertIntVal + +func fromConvertIntVal(v ConvertIntVal) (int64, error) { + return int64(v), nil +} + +func toConvertIntVal(i int64) (ConvertIntVal, error) { + return ConvertIntVal(i), nil +} + +type ConvertIntVal int64 + +// ConvertInt exercises a fixed-size (int64) convert shim, whose generated +// Msgsize must not declare an unused temporary (#446). +type ConvertInt struct { + Int ConvertIntVal +} diff --git a/_generated/convert_test.go b/_generated/convert_test.go index 7b67305e..5edd8ade 100644 --- a/_generated/convert_test.go +++ b/_generated/convert_test.go @@ -58,3 +58,23 @@ func TestConvertToMarshalError(t *testing.T) { t.Fatalf("expected conversion error, found %v", err.Error()) } } + +func TestConvertInt(t *testing.T) { + // #446: a fixed-size convert shim must generate a Msgsize that is an + // accurate constant and compiles (no unassigned temporary). + in := ConvertInt{Int: 42} + b, err := in.MarshalMsg(nil) + if err != nil { + t.Fatal(err) + } + if in.Msgsize() < len(b) { + t.Fatalf("Msgsize %d under-reports marshaled size %d", in.Msgsize(), len(b)) + } + var out ConvertInt + if _, err = out.UnmarshalMsg(b); err != nil { + t.Fatal(err) + } + if out != in { + t.Fatalf("round-trip mismatch: %v != %v", out, in) + } +} diff --git a/gen/size.go b/gen/size.go index 3efab37d..30594426 100644 --- a/gen/size.go +++ b/gen/size.go @@ -228,16 +228,23 @@ func (s *sizeGen) gBase(b *BaseElem) { return } if b.Convert && b.ShimMode == Convert { - s.state = add - vname := randIdent() - s.p.printf("\nvar %s %s", vname, b.BaseType()) - - // ensure we don't get "unused variable" warnings from outer slice iterations - s.p.printf("\n_ = %s", b.Varname()) - - s.p.printf("\ns += %s", basesizeExpr(b.Value, vname, b.BaseName())) - s.state = expr - + if fixedSize(b.Value) { + // A fixed-size base has a constant wire size, so there is no need + // for a temporary holding the converted value. Emitting an + // (unassigned) temporary left it unused, producing a "declared and + // not used" compile error in the generated Msgsize (#446). + s.addConstant(basesizeExpr(b.Value, "", b.BaseName())) + } else { + s.state = add + vname := randIdent() + s.p.printf("\nvar %s %s", vname, b.BaseType()) + + // ensure we don't get "unused variable" warnings from outer slice iterations + s.p.printf("\n_ = %s", b.Varname()) + + s.p.printf("\ns += %s", basesizeExpr(b.Value, vname, b.BaseName())) + s.state = expr + } } else { vname := b.Varname() if b.Convert { From fe1fe668b3e6feef927ddea553fa6401e0dd68a3 Mon Sep 17 00:00:00 2001 From: youdie006 Date: Mon, 31 Aug 2026 12:23:44 +0900 Subject: [PATCH 2/2] Apply review: flatten the fixed-size branch, widen the roundtrip Take both suggestions from the review: early-return in gBase so the non-fixed path is not indented, drop the bug narrative from the comments, and cover pointer, map and slice fields in the convert roundtrip. The added map and slice fields make ConvertInt non-comparable, so the test compares with reflect.DeepEqual. --- _generated/convert.go | 10 +++++++--- _generated/convert_test.go | 16 ++++++++++++---- gen/size.go | 22 ++++++++++------------ 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/_generated/convert.go b/_generated/convert.go index 7b9b4991..9f0d92bb 100644 --- a/_generated/convert.go +++ b/_generated/convert.go @@ -95,8 +95,12 @@ func toConvertIntVal(i int64) (ConvertIntVal, error) { type ConvertIntVal int64 -// ConvertInt exercises a fixed-size (int64) convert shim, whose generated -// Msgsize must not declare an unused temporary (#446). +// ConvertInt exercises a fixed-size (int64) convert shim. type ConvertInt struct { - Int ConvertIntVal + Int ConvertIntVal + Ptr *ConvertIntVal + Map map[string]ConvertIntVal + MapP map[string]*ConvertIntVal + Arr []ConvertIntVal + ArrP []*ConvertIntVal } diff --git a/_generated/convert_test.go b/_generated/convert_test.go index 5edd8ade..6440f745 100644 --- a/_generated/convert_test.go +++ b/_generated/convert_test.go @@ -2,6 +2,7 @@ package _generated import ( "bytes" + "reflect" "testing" "github.com/tinylib/msgp/msgp" @@ -60,9 +61,16 @@ func TestConvertToMarshalError(t *testing.T) { } func TestConvertInt(t *testing.T) { - // #446: a fixed-size convert shim must generate a Msgsize that is an - // accurate constant and compiles (no unassigned temporary). - in := ConvertInt{Int: 42} + // A fixed-size convert shim must report an accurate constant Msgsize. + v := ConvertIntVal(7) + in := ConvertInt{ + Int: 42, + Ptr: &v, + Map: map[string]ConvertIntVal{"a": 1}, + MapP: map[string]*ConvertIntVal{"b": &v}, + Arr: []ConvertIntVal{1, 2}, + ArrP: []*ConvertIntVal{&v}, + } b, err := in.MarshalMsg(nil) if err != nil { t.Fatal(err) @@ -74,7 +82,7 @@ func TestConvertInt(t *testing.T) { if _, err = out.UnmarshalMsg(b); err != nil { t.Fatal(err) } - if out != in { + if !reflect.DeepEqual(out, in) { t.Fatalf("round-trip mismatch: %v != %v", out, in) } } diff --git a/gen/size.go b/gen/size.go index 30594426..5b9639ae 100644 --- a/gen/size.go +++ b/gen/size.go @@ -230,21 +230,19 @@ func (s *sizeGen) gBase(b *BaseElem) { if b.Convert && b.ShimMode == Convert { if fixedSize(b.Value) { // A fixed-size base has a constant wire size, so there is no need - // for a temporary holding the converted value. Emitting an - // (unassigned) temporary left it unused, producing a "declared and - // not used" compile error in the generated Msgsize (#446). + // for a temporary holding the converted value. s.addConstant(basesizeExpr(b.Value, "", b.BaseName())) - } else { - s.state = add - vname := randIdent() - s.p.printf("\nvar %s %s", vname, b.BaseType()) + return + } + s.state = add + vname := randIdent() + s.p.printf("\nvar %s %s", vname, b.BaseType()) - // ensure we don't get "unused variable" warnings from outer slice iterations - s.p.printf("\n_ = %s", b.Varname()) + // ensure we don't get "unused variable" warnings from outer slice iterations + s.p.printf("\n_ = %s", b.Varname()) - s.p.printf("\ns += %s", basesizeExpr(b.Value, vname, b.BaseName())) - s.state = expr - } + s.p.printf("\ns += %s", basesizeExpr(b.Value, vname, b.BaseName())) + s.state = expr } else { vname := b.Varname() if b.Convert {