diff --git a/examples/magic.go b/examples/magic.go index 4b65c7c..92bb38f 100644 --- a/examples/magic.go +++ b/examples/magic.go @@ -126,9 +126,6 @@ func (h *fooHandler) HandleFoo(ctx context.Context, req *Request) (*Response, er return nil }, []string{"more", "messages", "sent"}, - cff.SliceEnd(func(context.Context) error { - return nil - }), ), cff.Map( func(ctx context.Context, key string, value string) error { @@ -144,9 +141,6 @@ func (h *fooHandler) HandleFoo(ctx context.Context, req *Request) (*Response, er return nil }, map[string]int{"a": 1, "b": 2, "c": 3}, - cff.MapEnd(func(context.Context) { - _ = fmt.Sprint("}") - }), ), ) return res, err diff --git a/examples/magic_gen.go b/examples/magic_gen.go index a3137b5..c005ce8 100644 --- a/examples/magic_gen.go +++ b/examples/magic_gen.go @@ -584,31 +584,23 @@ func (h *fooHandler) HandleFoo(ctx context.Context, req *Request) (*Response, er } /*line magic.go:128:4*/ _128_4 := []string{"more", "messages", "sent"} - /*line magic.go:129:17*/ - _129_17 := func(context.Context) error { - return nil - } - /*line magic.go:134:4*/ - _134_4 := func(ctx context.Context, key string, value string) error { + /*line magic.go:131:4*/ + _131_4 := func(ctx context.Context, key string, value string) error { _ = fmt.Sprintf("%q : %q", key, value) _, _ = ctx.Deadline() return nil } + /*line magic.go:136:4*/ + _136_4 := map[string]string{"key": "value"} /*line magic.go:139:4*/ - _139_4 := map[string]string{"key": "value"} - /*line magic.go:142:4*/ - _142_4 := func(ctx context.Context, key string, value int) error { + _139_4 := func(ctx context.Context, key string, value int) error { _ = fmt.Sprintf("%q: %v", key, value) return nil } - /*line magic.go:146:4*/ - _146_4 := map[string]int{"a": 1, "b": 2, "c": 3} - /*line magic.go:147:15*/ - _147_15 := func(context.Context) { - _ = fmt.Sprint("}") - } + /*line magic.go:143:4*/ + _143_4 := map[string]int{"a": 1, "b": 2, "c": 3} - /*line magic_gen.go:612*/ + /*line magic_gen.go:604*/ ctx := _88_3 emitter := cff.EmitterStack(_90_19, _91_19) @@ -849,7 +841,6 @@ func (h *fooHandler) HandleFoo(ctx context.Context, req *Request) (*Response, er // go.uber.org/cff/examples/magic.go:122:3 sliceTask11Slice := _128_4 - sliceTask11Jobs := make([]*cff.ScheduledJob, len(sliceTask11Slice)) for idx, val := range sliceTask11Slice { idx := idx val := val @@ -868,28 +859,13 @@ func (h *fooHandler) HandleFoo(ctx context.Context, req *Request) (*Response, er err = _123_4(ctx, idx, val) return } - sliceTask11Jobs[idx] = sched.Enqueue(ctx, cff.Job{ + sched.Enqueue(ctx, cff.Job{ Run: sliceTask11.fn, }) } - sched.Enqueue(ctx, cff.Job{ - Dependencies: sliceTask11Jobs, - Run: func(ctx context.Context) (err error) { - defer func() { - recovered := recover() - if recovered != nil { - err = fmt.Errorf("panic: %v", recovered) - } - }() - - err = _129_17(ctx) - return - }, - }) - - // go.uber.org/cff/examples/magic.go:133:3 - for key, val := range _139_4 { + // go.uber.org/cff/examples/magic.go:130:3 + for key, val := range _136_4 { key := key val := val mapTask12 := new(struct { @@ -905,7 +881,7 @@ func (h *fooHandler) HandleFoo(ctx context.Context, req *Request) (*Response, er } }() - err = _134_4(ctx, key, val) + err = _131_4(ctx, key, val) return } @@ -914,9 +890,8 @@ func (h *fooHandler) HandleFoo(ctx context.Context, req *Request) (*Response, er }) } - mapTask13Jobs := make([]*cff.ScheduledJob, 0, len(_146_4)) - // go.uber.org/cff/examples/magic.go:141:3 - for key, val := range _146_4 { + // go.uber.org/cff/examples/magic.go:138:3 + for key, val := range _143_4 { key := key val := val mapTask13 := new(struct { @@ -932,35 +907,21 @@ func (h *fooHandler) HandleFoo(ctx context.Context, req *Request) (*Response, er } }() - err = _142_4(ctx, key, val) + err = _139_4(ctx, key, val) return } - mapTask13Jobs = append(mapTask13Jobs, sched.Enqueue(ctx, cff.Job{ + sched.Enqueue(ctx, cff.Job{ Run: mapTask13.fn, - })) + }) } - sched.Enqueue(ctx, cff.Job{ - Dependencies: mapTask13Jobs, - Run: func(ctx context.Context) (err error) { - defer func() { - if recovered := recover(); recovered != nil { - err = fmt.Errorf("panic: %v", recovered) - } - }() - - _147_15(ctx) - return - }, - }) - if err := sched.Wait(ctx); err != nil { parallelEmitter.ParallelError(ctx, err) return err } parallelEmitter.ParallelSuccess(ctx) - return nil /*line magic.go:150*/ + return nil /*line magic.go:144*/ }() return res, err } diff --git a/examples/magic_v2.go b/examples/magic_v2.go index dbe2159..410300a 100644 --- a/examples/magic_v2.go +++ b/examples/magic_v2.go @@ -126,9 +126,6 @@ func (h *fooHandlerV2) HandleFoo(ctx context.Context, req *RequestV2) (*Response return nil }, []string{"more", "messages", "sent"}, - cff.SliceEnd(func(context.Context) error { - return nil - }), ), cff.Map( func(ctx context.Context, key string, value string) error { @@ -144,9 +141,6 @@ func (h *fooHandlerV2) HandleFoo(ctx context.Context, req *RequestV2) (*Response return nil }, map[string]int{"a": 1, "b": 2, "c": 3}, - cff.MapEnd(func(context.Context) { - _ = fmt.Sprint("}") - }), ), ) return res, err diff --git a/examples/magic_v2_gen.go b/examples/magic_v2_gen.go index 6cb4d18..fcd7cb7 100644 --- a/examples/magic_v2_gen.go +++ b/examples/magic_v2_gen.go @@ -127,9 +127,6 @@ func (h *fooHandlerV2) HandleFoo(ctx context.Context, req *RequestV2) (*Response return nil }, []string{"more", "messages", "sent"}, - cff.SliceEnd(func(context.Context) error { - return nil - }), ), cff.Map( func(ctx context.Context, key string, value string) error { @@ -145,9 +142,6 @@ func (h *fooHandlerV2) HandleFoo(ctx context.Context, req *RequestV2) (*Response return nil }, map[string]int{"a": 1, "b": 2, "c": 3}, - cff.MapEnd(func(context.Context) { - _ = fmt.Sprint("}") - }), ), ) return res, err diff --git a/internal/aquaregia_test.go b/internal/aquaregia_test.go index 9e14bc0..14df91d 100644 --- a/internal/aquaregia_test.go +++ b/internal/aquaregia_test.go @@ -229,6 +229,7 @@ var codeGenerateFailCases = map[string][]errorCase{ ErrorMatches: `"cff.SliceEnd" is an invalid option when "ContinueOnError" is used`, TestFuncs: []string{ "ParallelSliceEndWithContinueOnError", + "ParallelSliceEndWithContinueOnErrorAndInstrument", }, }, { @@ -393,7 +394,10 @@ var codeGenerateFailCases = map[string][]errorCase{ { File: "parallel.go", ErrorMatches: `"cff.MapEnd" is an invalid option when "ContinueOnError" is used`, - TestFuncs: []string{"ParallelMapEndWithContinueOnError"}, + TestFuncs: []string{ + "ParallelMapEndWithContinueOnError", + "ParallelMapEndWithContinueOnErrorAndInstrument", + }, }, }, "cycles": { diff --git a/internal/compile_parallel.go b/internal/compile_parallel.go index c58055f..cce969f 100644 --- a/internal/compile_parallel.go +++ b/internal/compile_parallel.go @@ -100,6 +100,18 @@ func (c *compiler) compileParallel(file *ast.File, call *ast.CallExpr) *parallel } c.validateParallelInstrument(parallel) + for _, s := range parallel.SliceTasks { + if s.SliceEndFn != nil && parallel.ContinueOnError != nil { + c.errf(c.nodePosition(parallel.Node), `"cff.SliceEnd" is an invalid option when "ContinueOnError" is used`) + } + } + + for _, m := range parallel.MapTasks { + if m.MapEndFn != nil && parallel.ContinueOnError != nil { + c.errf(c.nodePosition(parallel.Node), `"cff.MapEnd" is an invalid option when "ContinueOnError" is used`) + } + } + return parallel } @@ -119,18 +131,6 @@ func (c *compiler) validateParallelInstrument(p *parallel) { c.errf(c.nodePosition(p.Node), "cff.Instrument requires a cff.Emitter to be provided: use cff.WithEmitter") } } - - for _, s := range p.SliceTasks { - if s.SliceEndFn != nil && p.ContinueOnError != nil { - c.errf(c.nodePosition(p.Node), `"cff.SliceEnd" is an invalid option when "ContinueOnError" is used`) - } - } - - for _, m := range p.MapTasks { - if m.MapEndFn != nil && p.ContinueOnError != nil { - c.errf(c.nodePosition(p.Node), `"cff.MapEnd" is an invalid option when "ContinueOnError" is used`) - } - } } func (c *compiler) compileParallelTask(p *parallel, call ast.Expr, opts []ast.Expr) *parallelTask { diff --git a/internal/failing_tests/bad-inputs/parallel.go b/internal/failing_tests/bad-inputs/parallel.go index 08aa921..6744d54 100644 --- a/internal/failing_tests/bad-inputs/parallel.go +++ b/internal/failing_tests/bad-inputs/parallel.go @@ -357,6 +357,50 @@ func ParallelSliceEndWithInvalidReturn() { ) } +// ParallelSliceEndWithContinueOnErrorAndInstrument is a cff.Slice function +// that uses SliceEnd, ContinueOnError, and Instrument. +// +// This tests for a regression where SliceEnd-ContinueOnError incompatibility +// was not verified if Instrument was used. +func ParallelSliceEndWithContinueOnErrorAndInstrument() { + cff.Parallel(context.Background(), + cff.WithEmitter(cff.NopEmitter()), + cff.InstrumentParallel("myparallel"), + cff.ContinueOnError(true), + cff.Slice( + func(int, string) { + // stuff + }, + []string{"foo", "bar"}, + cff.SliceEnd(func() error { + return nil + }), + ), + ) +} + +// ParallelSliceEndWithContinueOnErrorAndInstrument is a cff.Slice function +// that uses SliceEnd, ContinueOnError, and Instrument. +// +// This tests for a regression where SliceEnd-ContinueOnError incompatibility +// was not verified if Instrument was used. +func ParallelMapEndWithContinueOnErrorAndInstrument() { + cff.Parallel(context.Background(), + cff.WithEmitter(cff.NopEmitter()), + cff.InstrumentParallel("myparallel"), + cff.ContinueOnError(true), + cff.Map( + func(string, string) { + // stuff + }, + map[string]string{"foo": "bar"}, + cff.MapEnd(func() error { + return nil + }), + ), + ) +} + // ParallelSliceWithTwoSliceEnds is a cff.Slice function that has more than // one cff.SliceEnd. func ParallelSliceWithTwoSliceEnds() {