From 66ff05a750f183af3cc3aeb0f230a7cd1b640e96 Mon Sep 17 00:00:00 2001 From: Qi Xiao Date: Sun, 21 Jan 2018 01:14:39 +0000 Subject: [PATCH] eval: Make ValuesOpFunc return an error. This addresses #570. --- eval/builtin_special.go | 37 ++++++++--- eval/compile_lvalue.go | 3 +- eval/compile_op.go | 39 ++++++++---- eval/compile_value.go | 134 ++++++++++++++++++++++++---------------- eval/frame.go | 9 ++- eval/unwrap.go | 4 +- 6 files changed, 150 insertions(+), 76 deletions(-) diff --git a/eval/builtin_special.go b/eval/builtin_special.go index 536ff332..158dc603 100644 --- a/eval/builtin_special.go +++ b/eval/builtin_special.go @@ -136,7 +136,10 @@ func newDelElementOp(ns, name string, begin, headEnd int, indexOps []ValuesOp) O return func(f *Frame) error { var indicies []types.Value for _, indexOp := range indexOps { - indexValues := indexOp.Exec(f) + indexValues, err := indexOp.Exec(f) + if err != nil { + return err + } if len(indexValues) != 1 { f.errorpf(indexOp.Begin, indexOp.End, "index must evaluate to a single value in argument to del") } @@ -189,7 +192,11 @@ func compileFn(cp *compiler, fn *parse.Form) OpFunc { // function. This step allows the definition of recursive // functions; the actual function will never be called. ec.local[varName] = vartypes.NewPtr(&BuiltinFn{"", nop}) - closure := op(ec)[0].(*Closure) + values, err := op(ec) + if err != nil { + return err + } + closure := values[0].(*Closure) closure.Op = makeFnOp(closure.Op) return ec.local[varName].Set(closure) } @@ -328,7 +335,10 @@ func compileAndOr(cp *compiler, fn *parse.Form, init, stopAt bool) OpFunc { return func(ec *Frame) error { var lastValue types.Value = types.Bool(init) for _, op := range argOps { - values := op.Exec(ec) + values, err := op.Exec(ec) + if err != nil { + return err + } for _, value := range values { if types.ToBool(value) == stopAt { ec.OutputChan() <- value @@ -370,7 +380,11 @@ func compileIf(cp *compiler, fn *parse.Form) OpFunc { } else_ := elseOp.execlambdaOp(ec) for i, condOp := range condOps { - if allTrue(condOp.Exec(ec.fork("if cond"))) { + condValues, err := condOp.Exec(ec.fork("if cond")) + if err != nil { + return err + } + if allTrue(condValues) { bodies[i].Call(ec.fork("if body"), NoArgs, NoOpts) return nil } @@ -395,11 +409,14 @@ func compileWhile(cp *compiler, fn *parse.Form) OpFunc { body := bodyOp.execlambdaOp(ec) for { - cond := condOp.Exec(ec.fork("while cond")) - if !allTrue(cond) { + condValues, err := condOp.Exec(ec.fork("while cond")) + if err != nil { + return err + } + if !allTrue(condValues) { break } - err := ec.fork("while").PCall(body, NoArgs, NoOpts) + err = ec.fork("while").PCall(body, NoArgs, NoOpts) if err != nil { exc := err.(*Exception) if exc.Cause == Continue { @@ -559,7 +576,11 @@ func (op ValuesOp) execlambdaOp(ec *Frame) Callable { return nil } - return op.Exec(ec)[0].(Callable) + values, err := op.Exec(ec) + if err != nil { + panic("must not be erroneous") + } + return values[0].(Callable) } // execMustOne executes the LValuesOp and raises an exception if it does not diff --git a/eval/compile_lvalue.go b/eval/compile_lvalue.go index 4685dbcf..b1bf3799 100644 --- a/eval/compile_lvalue.go +++ b/eval/compile_lvalue.go @@ -154,7 +154,8 @@ func (cp *compiler) lvalueElement(ns, name string, n *parse.Indexing) LValuesOpF indicies := make([]types.Value, len(indexOps)) for i, op := range indexOps { - values := op.Exec(ec) + values, err := op.Exec(ec) + maybeThrow(err) // TODO: Implement multi-indexing. if len(values) != 1 { throw(errors.New("multi indexing not implemented")) diff --git a/eval/compile_op.go b/eval/compile_op.go index f7f59d2f..e45d878d 100644 --- a/eval/compile_op.go +++ b/eval/compile_op.go @@ -191,8 +191,8 @@ func (cp *compiler) form(n *parse.Form) OpFunc { headOpFunc = variable(headStr + FnSuffix) } else { // Fall back to $e:head~. - headOpFunc = func(f *Frame) []types.Value { - return []types.Value{ExternalCmd{headStr}} + headOpFunc = func(f *Frame) ([]types.Value, error) { + return []types.Value{ExternalCmd{headStr}}, nil } } headOp = ValuesOp{headOpFunc, n.Head.Begin(), n.Head.End()} @@ -205,16 +205,20 @@ func (cp *compiler) form(n *parse.Form) OpFunc { } else { // Assignment form. varsOp, restOp := cp.lvaluesMulti(n.Vars) + // This cannot be replaced with newSeqValuesOp as it depends on the fact + // that argOps will be changed later. argsOp := ValuesOp{ - func(ec *Frame) []types.Value { - var vs []types.Value + func(ec *Frame) ([]types.Value, error) { + var values []types.Value for _, op := range argOps { - vs = append(vs, op.Exec(ec)...) + moreValues, err := op.Exec(ec) + if err != nil { + return nil, err + } + values = append(values, moreValues...) } - return vs - }, - -1, -1, - } + return values, nil + }, -1, -1} if len(argOps) > 0 { argsOp.Begin = argOps[0].Begin argsOp.End = argOps[len(argOps)-1].End @@ -300,13 +304,21 @@ func (cp *compiler) form(n *parse.Form) OpFunc { // args for _, argOp := range argOps { - args = append(args, argOp.Exec(ec)...) + moreArgs, err := argOp.Exec(ec) + if err != nil { + return err + } + args = append(args, moreArgs...) } } // opts // XXX This conversion should be avoided. - opts := optsOp(ec)[0].(types.Map) + optValues, err := optsOp(ec) + if err != nil { + return err + } + opts := optValues[0].(types.Map) convertedOpts := make(map[string]types.Value) var errOpt error opts.IteratePair(func(k, v types.Value) bool { @@ -371,7 +383,10 @@ func makeAssignmentOpFunc(variablesOp, restOp LValuesOp, valuesOp ValuesOp) OpFu defer fixNilVariables(variables) defer fixNilVariables(rest) - values := valuesOp.Exec(ec) + values, err := valuesOp.Exec(ec) + if err != nil { + return err + } if len(rest) > 1 { return ErrMoreThanOneRest diff --git a/eval/compile_value.go b/eval/compile_value.go index 531cbed6..99e03bf1 100644 --- a/eval/compile_value.go +++ b/eval/compile_value.go @@ -13,6 +13,7 @@ import ( "github.com/elves/elvish/eval/types" "github.com/elves/elvish/glob" "github.com/elves/elvish/parse" + "github.com/elves/elvish/util" "github.com/xiaq/persistent/hashmap" ) @@ -25,10 +26,10 @@ type ValuesOp struct { } // ValuesOpFunc is the body of ValuesOp. -type ValuesOpFunc func(*Frame) []types.Value +type ValuesOpFunc func(*Frame) ([]types.Value, error) // Exec executes a ValuesOp and produces Value's. -func (op ValuesOp) Exec(ec *Frame) []types.Value { +func (op ValuesOp) Exec(ec *Frame) ([]types.Value, error) { ec.begin, ec.end = op.Begin, op.End return op.Func(ec) } @@ -44,8 +45,12 @@ func (cp *compiler) compound(n *parse.Compound) ValuesOpFunc { if n.Indexings[0].Head.Type == parse.Tilde { // A lone ~. if len(n.Indexings) == 1 { - return func(ec *Frame) []types.Value { - return []types.Value{types.String(mustGetHome(""))} + return func(ec *Frame) ([]types.Value, error) { + home, err := util.GetHome("") + if err != nil { + return nil, err + } + return []types.Value{types.String(home)}, nil } } tilde = true @@ -54,14 +59,20 @@ func (cp *compiler) compound(n *parse.Compound) ValuesOpFunc { ops := cp.indexingOps(indexings) - return func(ec *Frame) []types.Value { + return func(ec *Frame) ([]types.Value, error) { // Accumulator. - vs := ops[0].Exec(ec) + vs, err := ops[0].Exec(ec) + if err != nil { + return nil, err + } // Logger.Printf("concatenating %v with %d more", vs, len(ops)-1) for _, op := range ops[1:] { - us := op.Exec(ec) + us, err := op.Exec(ec) + if err != nil { + return nil, err + } vs = outerProduct(vs, us, cat) // Logger.Printf("with %v => %v", us, vs) } @@ -91,7 +102,7 @@ func (cp *compiler) compound(n *parse.Compound) ValuesOpFunc { } vs = newvs } - return vs + return vs, nil } } @@ -185,20 +196,7 @@ func doTilde(v types.Value) types.Value { } func (cp *compiler) array(n *parse.Array) ValuesOpFunc { - return catValuesOps(cp.compoundOps(n.Compounds)) -} - -func catValuesOps(ops []ValuesOp) ValuesOpFunc { - return func(ec *Frame) []types.Value { - // Use number of compound expressions as an estimation of the number - // of values - vs := make([]types.Value, 0, len(ops)) - for _, op := range ops { - us := op.Exec(ec) - vs = append(vs, us...) - } - return vs - } + return newSeqValuesOp(cp.compoundOps(n.Compounds)) } func (cp *compiler) indexing(n *parse.Indexing) ValuesOpFunc { @@ -209,31 +207,39 @@ func (cp *compiler) indexing(n *parse.Indexing) ValuesOpFunc { headOp := cp.primaryOp(n.Head) indexOps := cp.arrayOps(n.Indicies) - return func(ec *Frame) []types.Value { - vs := headOp.Exec(ec) + return func(ec *Frame) ([]types.Value, error) { + vs, err := headOp.Exec(ec) + if err != nil { + return nil, err + } for _, indexOp := range indexOps { - indicies := indexOp.Exec(ec) + indicies, err := indexOp.Exec(ec) + if err != nil { + return nil, err + } newvs := make([]types.Value, 0, len(vs)*len(indicies)) for _, v := range vs { indexer, ok := v.(types.Indexer) if !ok { - throwf("a %s not indexable", v.Kind()) + return nil, fmt.Errorf("a %s not indexable", v.Kind()) } for _, index := range indicies { result, err := indexer.Index(index) - maybeThrow(err) + if err != nil { + return nil, err + } newvs = append(newvs, result) } } vs = newvs } - return vs + return vs, nil } } func literalValues(v ...types.Value) ValuesOpFunc { - return func(e *Frame) []types.Value { - return v + return func(e *Frame) ([]types.Value, error) { + return v, nil } } @@ -243,21 +249,21 @@ func literalStr(text string) ValuesOpFunc { func variable(qname string) ValuesOpFunc { explode, ns, name := ParseVariable(qname) - return func(ec *Frame) []types.Value { + return func(ec *Frame) ([]types.Value, error) { variable := ec.ResolveVar(ns, name) if variable == nil { - throwf("variable $%s not found", qname) + return nil, fmt.Errorf("variable $%s not found", qname) } value := variable.Get() if explode { iterator, ok := value.(types.Iterator) if !ok { // Use qname[1:] to skip the leading "@" - throwf("variable $%s (kind %s) cannot be exploded", qname[1:], value.Kind()) + return nil, fmt.Errorf("variable $%s (kind %s) cannot be exploded", qname[1:], value.Kind()) } - return types.CollectFromIterator(iterator) + return types.CollectFromIterator(iterator), nil } - return []types.Value{value} + return []types.Value{value}, nil } } @@ -278,9 +284,7 @@ func (cp *compiler) primary(n *parse.Primary) ValuesOpFunc { } vs := []types.Value{ GlobPattern{glob.Pattern{[]glob.Segment{seg}, ""}, 0, nil}} - return func(ec *Frame) []types.Value { - return vs - } + return literalValues(vs...) case parse.Tilde: cp.errorf("compiler bug: Tilde not handled in .compound") return literalStr("~") @@ -305,27 +309,31 @@ func (cp *compiler) primary(n *parse.Primary) ValuesOpFunc { func (cp *compiler) list(n *parse.Primary) ValuesOpFunc { // TODO(xiaq): Use Vector.Cons to build the list, instead of building a // slice and converting to Vector. - op := catValuesOps(cp.compoundOps(n.Elements)) - return func(ec *Frame) []types.Value { - return []types.Value{types.MakeList(op(ec)...)} + op := newSeqValuesOp(cp.compoundOps(n.Elements)) + return func(ec *Frame) ([]types.Value, error) { + values, err := op(ec) + if err != nil { + return nil, err + } + return []types.Value{types.MakeList(values...)}, nil } } func (cp *compiler) exceptionCapture(n *parse.Chunk) ValuesOpFunc { op := cp.chunkOp(n) - return func(ec *Frame) []types.Value { + return func(ec *Frame) ([]types.Value, error) { err := ec.PEval(op) if err == nil { - return []types.Value{OK} + return []types.Value{OK}, nil } - return []types.Value{err.(*Exception)} + return []types.Value{err.(*Exception)}, nil } } func (cp *compiler) outputCapture(n *parse.Primary) ValuesOpFunc { op := cp.chunkOp(n.Chunk) - return func(ec *Frame) []types.Value { - return captureOutput(ec, op) + return func(ec *Frame) ([]types.Value, error) { + return captureOutput(ec, op), nil } } @@ -481,7 +489,7 @@ func (cp *compiler) lambda(n *parse.Primary) ValuesOpFunc { srcMeta := cp.srcMeta - return func(ec *Frame) []types.Value { + return func(ec *Frame) ([]types.Value, error) { evCapture := make(Ns) for name := range capture { evCapture[name] = ec.ResolveVar("", name) @@ -492,7 +500,7 @@ func (cp *compiler) lambda(n *parse.Primary) ValuesOpFunc { optDefaults[i] = defaultValue } // XXX(xiaq): Capture uses. - return []types.Value{&Closure{argNames, restArgName, optNames, optDefaults, op, evCapture, srcMeta}} + return []types.Value{&Closure{argNames, restArgName, optNames, optDefaults, op, evCapture, srcMeta}}, nil } } @@ -515,11 +523,17 @@ func (cp *compiler) mapPairs(pairs []*parse.MapPair) ValuesOpFunc { } begins[i], ends[i] = pair.Begin(), pair.End() } - return func(ec *Frame) []types.Value { + return func(ec *Frame) ([]types.Value, error) { m := hashmap.Empty for i := 0; i < npairs; i++ { - keys := keysOps[i].Exec(ec) - values := valuesOps[i].Exec(ec) + keys, err := keysOps[i].Exec(ec) + if err != nil { + return nil, err + } + values, err := valuesOps[i].Exec(ec) + if err != nil { + return nil, err + } if len(keys) != len(values) { ec.errorpf(begins[i], ends[i], "%d keys but %d values", len(keys), len(values)) @@ -528,7 +542,7 @@ func (cp *compiler) mapPairs(pairs []*parse.MapPair) ValuesOpFunc { m = m.Assoc(key, values[j]) } } - return []types.Value{types.NewMap(m)} + return []types.Value{types.NewMap(m)}, nil } } @@ -536,5 +550,19 @@ func (cp *compiler) braced(n *parse.Primary) ValuesOpFunc { ops := cp.compoundOps(n.Braced) // TODO: n.IsRange // isRange := n.IsRange - return catValuesOps(ops) + return newSeqValuesOp(ops) +} + +func newSeqValuesOp(ops []ValuesOp) ValuesOpFunc { + return func(ec *Frame) ([]types.Value, error) { + var values []types.Value + for _, op := range ops { + moreValues, err := op.Exec(ec) + if err != nil { + return nil, err + } + values = append(values, moreValues...) + } + return values, nil + } } diff --git a/eval/frame.go b/eval/frame.go index d1c1fe98..87939227 100644 --- a/eval/frame.go +++ b/eval/frame.go @@ -118,7 +118,14 @@ func (ec *Frame) fork(name string) *Frame { // wrapped in an Error. func (ec *Frame) PEval(op Op) (err error) { defer catch(&err, ec) - return op.Exec(ec) + e := op.Exec(ec) + if e != nil { + if exc, ok := e.(*Exception); ok { + return exc + } + return ec.makeException(e) + } + return nil } func (ec *Frame) PCall(f Callable, args []types.Value, opts map[string]types.Value) (err error) { diff --git a/eval/unwrap.go b/eval/unwrap.go index 4ae9f380..0481063b 100644 --- a/eval/unwrap.go +++ b/eval/unwrap.go @@ -41,7 +41,9 @@ func (ctx *Frame) Unwrap(desc string, begin, end int, vs []types.Value) ValuesUn // ExecAndUnwrap executes a ValuesOp and creates an Unwrapper for the obtained // values. func (ctx *Frame) ExecAndUnwrap(desc string, op ValuesOp) ValuesUnwrapper { - return ctx.Unwrap(desc, op.Begin, op.End, op.Exec(ctx)) + values, err := op.Exec(ctx) + maybeThrow(err) + return ctx.Unwrap(desc, op.Begin, op.End, values) } // One unwraps the value to be exactly one value.