Skip to content

Commit

Permalink
roachpb: remove SetInner in favor of MustSetInner
Browse files Browse the repository at this point in the history
As of a recent commit, `ErrorDetail.SetInner` became unused, and
we can switch to a `MustSetInner` pattern for `ErrorDetail`. Since
the codegen involved is shared with {Request,Response}Union, those
lose the `SetInner` setter as well; we were always asserting on
the returned bool there anyway so this isn't changing anything.

Release note: None
  • Loading branch information
tbg committed Nov 12, 2020
1 parent 6d2ce39 commit b5085d2
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 50 deletions.
2 changes: 1 addition & 1 deletion pkg/kv/kvclient/kvcoord/txn_interceptor_committer.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (tc *txnCommitter) SendLocked(
// Make a copy of the EndTxn, since we're going to change it below to
// disable the parallel commit.
etCpy := *et
ba.Requests[len(ba.Requests)-1].SetInner(&etCpy)
ba.Requests[len(ba.Requests)-1].MustSetInner(&etCpy)
et = &etCpy
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func (sr *txnSpanRefresher) SendLocked(
isReissue := et.DeprecatedCanCommitAtHigherTimestamp
if isReissue {
etCpy := *et
ba.Requests[len(ba.Requests)-1].SetInner(&etCpy)
ba.Requests[len(ba.Requests)-1].MustSetInner(&etCpy)
et = &etCpy
}
et.DeprecatedCanCommitAtHigherTimestamp = ba.CanForwardReadTimestamp
Expand Down
20 changes: 0 additions & 20 deletions pkg/roachpb/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -597,26 +597,6 @@ func (sr *ReverseScanResponse) Verify(req Request) error {
return nil
}

// MustSetInner sets the Request contained in the union. It panics if the
// request is not recognized by the union type. The RequestUnion is reset
// before being repopulated.
func (ru *RequestUnion) MustSetInner(args Request) {
ru.Reset()
if !ru.SetInner(args) {
panic(errors.AssertionFailedf("%T excludes %T", ru, args))
}
}

// MustSetInner sets the Response contained in the union. It panics if the
// response is not recognized by the union type. The ResponseUnion is reset
// before being repopulated.
func (ru *ResponseUnion) MustSetInner(reply Response) {
ru.Reset()
if !ru.SetInner(reply) {
panic(errors.AssertionFailedf("%T excludes %T", ru, reply))
}
}

// Method implements the Request interface.
func (*GetRequest) Method() Method { return Get }

Expand Down
2 changes: 1 addition & 1 deletion pkg/roachpb/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ func TestMustSetInner(t *testing.T) {
req := RequestUnion{}
res := ResponseUnion{}

// GetRequest is checked first in the generated code for SetInner.
// GetRequest is checked first in the generated code for MustSetInner.
req.MustSetInner(&GetRequest{})
res.MustSetInner(&GetResponse{})
req.MustSetInner(&EndTxnRequest{})
Expand Down
24 changes: 12 additions & 12 deletions pkg/roachpb/batch_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 4 additions & 6 deletions pkg/roachpb/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ func (e *internalError) Error() string {
}

// ErrorDetailInterface is an interface for each error detail.
// These must not be implemented by anything other than our protobuf-backed error details
// as we rely on a 1:1 correspondence between the interface and what can be stored via
// `Error.SetDetail`.
type ErrorDetailInterface interface {
error
protoutil.Message
Expand Down Expand Up @@ -307,12 +310,7 @@ func (e *Error) SetDetail(detail ErrorDetailInterface) {
} else {
e.TransactionRestart = TransactionRestart_NONE
}
// If the specific error type exists in the detail union, set it.
if !e.Detail.SetInner(detail) {
if e.TransactionRestart != TransactionRestart_NONE {
panic(errors.AssertionFailedf("transactionRestartError %T must be an ErrorDetail", detail))
}
}
e.Detail.MustSetInner(detail)
e.checkTxnStatusValid()
}

Expand Down
18 changes: 9 additions & 9 deletions pkg/roachpb/gen_batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,11 @@ func (ru %[1]s) GetInner() %[2]s {
`)
}

func genSetInner(w io.Writer, unionName, variantName string, variants []variantInfo) {
func genMustSetInner(w io.Writer, unionName, variantName string, variants []variantInfo) {
fmt.Fprintf(w, `
// SetInner sets the %[2]s in the union.
func (ru *%[1]s) SetInner(r %[2]s) bool {
// MustSetInner sets the %[2]s in the union.
func (ru *%[1]s) MustSetInner(r %[2]s) {
ru.Reset()
var union is%[1]s_Value
switch t := r.(type) {
`, unionName, variantName)
Expand All @@ -123,10 +124,9 @@ func (ru *%[1]s) SetInner(r %[2]s) bool {
}

fmt.Fprint(w, ` default:
return false
panic(fmt.Sprintf("unsupported type %T for %T", r, ru))
}
ru.Value = union
return true
}
`)
}
Expand Down Expand Up @@ -160,10 +160,10 @@ import (
genGetInner(f, "RequestUnion", "Request", reqVariants)
genGetInner(f, "ResponseUnion", "Response", resVariants)

// Generate SetInner methods.
genSetInner(f, "ErrorDetail", "error", errVariants)
genSetInner(f, "RequestUnion", "Request", reqVariants)
genSetInner(f, "ResponseUnion", "Response", resVariants)
// Generate MustSetInner methods.
genMustSetInner(f, "ErrorDetail", "error", errVariants)
genMustSetInner(f, "RequestUnion", "Request", reqVariants)
genMustSetInner(f, "ResponseUnion", "Response", resVariants)

fmt.Fprintf(f, `
type reqCounts [%d]int32
Expand Down

0 comments on commit b5085d2

Please sign in to comment.