From af409b775c554af6dc7b7e478ab55228a91eff79 Mon Sep 17 00:00:00 2001 From: Håvard Pettersen Date: Tue, 31 Oct 2017 13:35:24 +0000 Subject: remove let --- eval/src/apps/eval_expr/eval_expr.cpp | 1 - eval/src/tests/eval/function/function_test.cpp | 38 ----------- eval/src/tests/eval/node_types/node_types_test.cpp | 10 --- eval/src/vespa/eval/eval/basic_nodes.cpp | 1 - eval/src/vespa/eval/eval/basic_nodes.h | 60 ++--------------- eval/src/vespa/eval/eval/function.cpp | 75 +++------------------- eval/src/vespa/eval/eval/gbdt.cpp | 4 +- eval/src/vespa/eval/eval/interpreted_function.cpp | 34 +--------- eval/src/vespa/eval/eval/interpreted_function.h | 1 - eval/src/vespa/eval/eval/key_gen.cpp | 1 - eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp | 19 +----- eval/src/vespa/eval/eval/node_types.cpp | 63 ++---------------- eval/src/vespa/eval/eval/node_visitor.h | 2 - eval/src/vespa/eval/eval/param_usage.cpp | 6 +- eval/src/vespa/eval/eval/test/eval_spec.cpp | 14 ---- eval/src/vespa/eval/eval/test/eval_spec.h | 2 - eval/src/vespa/eval/eval/vm_forest.cpp | 4 +- 17 files changed, 33 insertions(+), 302 deletions(-) (limited to 'eval') diff --git a/eval/src/apps/eval_expr/eval_expr.cpp b/eval/src/apps/eval_expr/eval_expr.cpp index 5200e37fe5e..2e1f7f7fdcb 100644 --- a/eval/src/apps/eval_expr/eval_expr.cpp +++ b/eval/src/apps/eval_expr/eval_expr.cpp @@ -12,7 +12,6 @@ int main(int argc, char **argv) { fprintf(stderr, "usage: %s \n", argv[0]); fprintf(stderr, " the expression must be self-contained (no arguments)\n"); fprintf(stderr, " quote the expression to make it a single parameter\n"); - fprintf(stderr, " use let to simulate parameters: let(x, 1, x + 3)\n"); return 1; } Function function = Function::parse({}, argv[1]); diff --git a/eval/src/tests/eval/function/function_test.cpp b/eval/src/tests/eval/function/function_test.cpp index eae58b04178..75a6df41b50 100644 --- a/eval/src/tests/eval/function/function_test.cpp +++ b/eval/src/tests/eval/function/function_test.cpp @@ -345,15 +345,6 @@ TEST("require that If children can be accessed") { EXPECT_EQUAL(3.0, root.get_child(2).get_const_value()); } -TEST("require that Let children can be accessed") { - Function f = Function::parse("let(a,1,2)"); - const Node &root = f.root(); - EXPECT_TRUE(!root.is_leaf()); - ASSERT_EQUAL(2u, root.num_children()); - EXPECT_EQUAL(1.0, root.get_child(0).get_const_value()); - EXPECT_EQUAL(2.0, root.get_child(1).get_const_value()); -} - TEST("require that Operator children can be accessed") { Function f = Function::parse("1+2"); const Node &root = f.root(); @@ -395,7 +386,6 @@ TEST("require that children can be detached") { EXPECT_EQUAL(1u, detach_from_root("-a")); EXPECT_EQUAL(1u, detach_from_root("!a")); EXPECT_EQUAL(3u, detach_from_root("if(1,2,3)")); - EXPECT_EQUAL(2u, detach_from_root("let(a,1,a)")); EXPECT_EQUAL(5u, detach_from_root("[1,2,3,4,5]")); EXPECT_EQUAL(2u, detach_from_root("a+b")); EXPECT_EQUAL(1u, detach_from_root("isNan(a)")); @@ -493,14 +483,6 @@ TEST("require that inverted parameter is not param") { EXPECT_TRUE(!Function::parse("-x").root().is_param()); } -TEST("require that let references are not params") { - Function fun = Function::parse("let(foo,bar,foo)"); - auto let = as(fun.root()); - ASSERT_TRUE(let); - EXPECT_TRUE(let->value().is_param()); - EXPECT_TRUE(!let->expr().is_param()); -} - TEST("require that number is const, but not param") { EXPECT_TRUE(Function::parse("123").root().is_const()); EXPECT_TRUE(!Function::parse("123").root().is_param()); @@ -549,10 +531,6 @@ TEST("require that calls are cost if all parameters are const") { EXPECT_TRUE(Function::parse("max(1,2)").root().is_const()); } -TEST("require that const let is not const") { - EXPECT_TRUE(!Function::parse("let(a,1,a)").root().is_const()); -} - //----------------------------------------------------------------------------- TEST("require that feature less than constant is tree if children are trees or constants") { @@ -693,18 +671,6 @@ TEST("require that unknown function that is valid parameter works as expected wi EXPECT_EQUAL("[z(x)]...[unknown symbol: 'z(x)']...[+y]", Function::parse(params, "z(x)+y", MySymbolExtractor({'(', ')'})).dump()); } -TEST("require that custom symbol extractor is only invoked for tokens that must be parameters") { - MySymbolExtractor my_extractor; - EXPECT_EQUAL(0u, Function::parse("max(1,2)", my_extractor).num_params()); - EXPECT_EQUAL(0u, Function::parse("max(let(a,1,a),2)", my_extractor).num_params()); - ASSERT_EQUAL(1u, Function::parse("max(let(a,1,b),2)", my_extractor).num_params()); - EXPECT_EQUAL(1u, my_extractor.invoke_count); - EXPECT_EQUAL("b", Function::parse("max(let(a,1,b),2)", my_extractor).param_name(0)); - EXPECT_EQUAL(2u, my_extractor.invoke_count); - EXPECT_EQUAL("[bogus]...[invalid operator: '(']...[(1,2)]", Function::parse("bogus(1,2)", my_extractor).dump()); - EXPECT_EQUAL(3u, my_extractor.invoke_count); -} - //----------------------------------------------------------------------------- void verify_error(const vespalib::string &expr, const vespalib::string &expected_error) { @@ -779,10 +745,6 @@ TEST("require that outer parameters are hidden within a lambda") { verify_error("map(x,f(a)(y))", "[map(x,f(a)(y]...[unknown symbol: 'y']...[))]"); } -TEST("require that outer let bindings are hidden within a lambda") { - verify_error("let(b,x,map(b,f(a)(b)))", "[let(b,x,map(b,f(a)(b]...[unknown symbol: 'b']...[)))]"); -} - //----------------------------------------------------------------------------- TEST("require that tensor reduce can be parsed") { diff --git a/eval/src/tests/eval/node_types/node_types_test.cpp b/eval/src/tests/eval/node_types/node_types_test.cpp index f6afa543402..e8009447793 100644 --- a/eval/src/tests/eval/node_types/node_types_test.cpp +++ b/eval/src/tests/eval/node_types/node_types_test.cpp @@ -108,16 +108,6 @@ TEST("require that if resolves to the appropriate type") { TEST_DO(verify("if(double,any,double)", "any")); } -TEST("require that let expressions propagate type correctly") { - TEST_DO(verify("let(a,10,a)", "double")); - TEST_DO(verify("let(a,double,a)", "double")); - TEST_DO(verify("let(a,any,a)", "any")); - TEST_DO(verify("let(a,error,a)", "error")); - TEST_DO(verify("let(a,tensor,let(b,double,a))", "tensor")); - TEST_DO(verify("let(a,tensor,let(b,double,b))", "double")); - TEST_DO(verify("let(a,tensor,let(b,a,b))", "tensor")); -} - TEST("require that set membership resolves to double unless error") { TEST_DO(verify("1 in [1,2,3]", "double")); TEST_DO(verify("1 in [tensor,tensor,tensor]", "double")); diff --git a/eval/src/vespa/eval/eval/basic_nodes.cpp b/eval/src/vespa/eval/eval/basic_nodes.cpp index 2222b5488c5..50db7370a66 100644 --- a/eval/src/vespa/eval/eval/basic_nodes.cpp +++ b/eval/src/vespa/eval/eval/basic_nodes.cpp @@ -64,7 +64,6 @@ void Array ::accept(NodeVisitor &visitor) const { visitor.visit(*this); } void Neg ::accept(NodeVisitor &visitor) const { visitor.visit(*this); } void Not ::accept(NodeVisitor &visitor) const { visitor.visit(*this); } void If ::accept(NodeVisitor &visitor) const { visitor.visit(*this); } -void Let ::accept(NodeVisitor &visitor) const { visitor.visit(*this); } void Error ::accept(NodeVisitor &visitor) const { visitor.visit(*this); } vespalib::string diff --git a/eval/src/vespa/eval/eval/basic_nodes.h b/eval/src/vespa/eval/eval/basic_nodes.h index dd4fccaa1ae..3f4c21be810 100644 --- a/eval/src/vespa/eval/eval/basic_nodes.h +++ b/eval/src/vespa/eval/eval/basic_nodes.h @@ -35,9 +35,8 @@ namespace nodes { **/ struct DumpContext { const std::vector ¶m_names; - std::vector let_names; DumpContext(const std::vector ¶m_names_in) - : param_names(param_names_in), let_names() {} + : param_names(param_names_in) {} }; /** @@ -118,23 +117,14 @@ public: class Symbol : public Leaf { private: - int _id; + size_t _id; public: - static const int UNDEF = std::numeric_limits::max(); - explicit Symbol(int id_in) : _id(id_in) {} - int id() const { return _id; } - bool is_param() const override { - return (_id >= 0); - } + explicit Symbol(size_t id_in) : _id(id_in) {} + bool is_param() const override { return true; } + size_t id() const { return _id; } vespalib::string dump(DumpContext &ctx) const override { - if (_id >= 0) { // param value - assert(size_t(_id) < ctx.param_names.size()); - return ctx.param_names[_id]; - } else { // let binding - int let_offset = -(_id + 1); - assert(size_t(let_offset) < ctx.let_names.size()); - return ctx.let_names[let_offset]; - } + assert(size_t(_id) < ctx.param_names.size()); + return ctx.param_names[_id]; } void accept(NodeVisitor &visitor) const override; }; @@ -294,42 +284,6 @@ public: void accept(NodeVisitor &visitor) const override; }; -class Let : public Node { -private: - vespalib::string _name; - Node_UP _value; - Node_UP _expr; -public: - Let(const vespalib::string &name_in, Node_UP value_in, Node_UP expr_in) - : _name(name_in), _value(std::move(value_in)), _expr(std::move(expr_in)) {} - const vespalib::string &name() const { return _name; } - const Node &value() const { return *_value; } - const Node &expr() const { return *_expr; } - size_t num_children() const override { return (_value && _expr) ? 2 : 0; } - const Node &get_child(size_t idx) const override { - assert(idx < 2); - return (idx == 0) ? value() : expr(); - } - void detach_children(NodeHandler &handler) override { - handler.handle(std::move(_value)); - handler.handle(std::move(_expr)); - } - vespalib::string dump(DumpContext &ctx) const override { - vespalib::string str; - str += "let("; - str += _name; - str += ","; - str += _value->dump(ctx); - str += ","; - ctx.let_names.push_back(_name); - str += _expr->dump(ctx); - ctx.let_names.pop_back(); - str += ")"; - return str; - } - void accept(NodeVisitor &visitor) const override; -}; - class Error : public Leaf { private: vespalib::string _message; diff --git a/eval/src/vespa/eval/eval/function.cpp b/eval/src/vespa/eval/eval/function.cpp index 58d61d33ea6..0843baa700c 100644 --- a/eval/src/vespa/eval/eval/function.cpp +++ b/eval/src/vespa/eval/eval/function.cpp @@ -100,39 +100,12 @@ struct ImplicitParams : Params { class ResolveContext { private: - const Params &_params; - const SymbolExtractor *_symbol_extractor; - std::vector _let_names; + const Params &_params; + const SymbolExtractor *_symbol_extractor; public: ResolveContext(const Params ¶ms, const SymbolExtractor *symbol_extractor) - : _params(params), _symbol_extractor(symbol_extractor), _let_names() {} - - void push_let_name(const vespalib::string &name) { - _let_names.push_back(name); - } - - void pop_let_name() { - assert(!_let_names.empty()); - _let_names.pop_back(); - } - - int resolve_let_name(const vespalib::string &name) const { - for (int i = (int(_let_names.size()) - 1); i >= 0; --i) { - if (name == _let_names[i]) { - return -(i + 1); - } - } - return nodes::Symbol::UNDEF; - } - - int resolve_param(const vespalib::string &name) const { - size_t param_id = _params.resolve(name); - if (param_id == Params::UNDEF) { - return nodes::Symbol::UNDEF; - } - return param_id; - } - + : _params(params), _symbol_extractor(symbol_extractor) {} + size_t resolve_param(const vespalib::string &name) const { return _params.resolve(name); } const SymbolExtractor *symbol_extractor() const { return _symbol_extractor; } }; @@ -247,19 +220,7 @@ public: } } - void push_let_binding(const vespalib::string &name) { - resolver().push_let_name(name); - } - - void pop_let_binding() { - resolver().pop_let_name(); - } - - int resolve_let_ref(const vespalib::string &name) const { - return resolver().resolve_let_name(name); - } - - int resolve_parameter(const vespalib::string &name) const { + size_t resolve_parameter(const vespalib::string &name) const { return resolver().resolve_param(name); } @@ -472,20 +433,6 @@ void parse_if(ParseContext &ctx) { ctx.push_expression(Node_UP(new nodes::If(std::move(cond), std::move(true_expr), std::move(false_expr), p_true))); } -void parse_let(ParseContext &ctx) { - vespalib::string name = get_ident(ctx, false); - ctx.skip_spaces(); - ctx.eat(','); - parse_expression(ctx); - Node_UP value = ctx.pop_expression(); - ctx.eat(','); - ctx.push_let_binding(name); - parse_expression(ctx); - Node_UP expr = ctx.pop_expression(); - ctx.pop_let_binding(); - ctx.push_expression(Node_UP(new nodes::Let(name, std::move(value), std::move(expr)))); -} - void parse_call(ParseContext &ctx, Call_UP call) { for (size_t i = 0; i < call->num_params(); ++i) { if (i > 0) { @@ -648,8 +595,6 @@ bool try_parse_call(ParseContext &ctx, const vespalib::string &name) { ctx.eat('('); if (name == "if") { parse_if(ctx); - } else if (name == "let") { - parse_let(ctx); } else { Call_UP call = nodes::CallRepo::instance().create(name); if (call.get() != nullptr) { @@ -677,11 +622,7 @@ bool try_parse_call(ParseContext &ctx, const vespalib::string &name) { return false; } -int parse_symbol(ParseContext &ctx, vespalib::string &name, ParseContext::InputMark before_name) { - int id = ctx.resolve_let_ref(name); - if (id != nodes::Symbol::UNDEF) { - return id; - } +size_t parse_symbol(ParseContext &ctx, vespalib::string &name, ParseContext::InputMark before_name) { ctx.extract_symbol(name, before_name); return ctx.resolve_parameter(name); } @@ -690,10 +631,10 @@ void parse_symbol_or_call(ParseContext &ctx) { ParseContext::InputMark before_name = ctx.get_input_mark(); vespalib::string name = get_ident(ctx, true); if (!try_parse_call(ctx, name)) { - int id = parse_symbol(ctx, name, before_name); + size_t id = parse_symbol(ctx, name, before_name); if (name.empty()) { ctx.fail("missing value"); - } else if (id == nodes::Symbol::UNDEF) { + } else if (id == Params::UNDEF) { ctx.fail(make_string("unknown symbol: '%s'", name.c_str())); } else { ctx.push_expression(Node_UP(new nodes::Symbol(id))); diff --git a/eval/src/vespa/eval/eval/gbdt.cpp b/eval/src/vespa/eval/eval/gbdt.cpp index 2d00eab1af0..cffd8bdb0c3 100644 --- a/eval/src/vespa/eval/eval/gbdt.cpp +++ b/eval/src/vespa/eval/eval/gbdt.cpp @@ -65,13 +65,13 @@ TreeStats::traverse(const nodes::Node &node, size_t depth, size_t &sum_path) { auto in = nodes::as(if_node->cond()); if (less) { auto symbol = nodes::as(less->lhs()); - assert(symbol && (symbol->id() >= 0)); + assert(symbol); num_params = std::max(num_params, size_t(symbol->id() + 1)); ++num_less_checks; } else { assert(in); auto symbol = nodes::as(in->lhs()); - assert(symbol && (symbol->id() >= 0)); + assert(symbol); num_params = std::max(num_params, size_t(symbol->id() + 1)); ++num_in_checks; auto array = nodes::as(in->rhs()); diff --git a/eval/src/vespa/eval/eval/interpreted_function.cpp b/eval/src/vespa/eval/eval/interpreted_function.cpp index 82aa71893e1..caf71fcb68b 100644 --- a/eval/src/vespa/eval/eval/interpreted_function.cpp +++ b/eval/src/vespa/eval/eval/interpreted_function.cpp @@ -49,10 +49,6 @@ void op_load_param(State &state, uint64_t param) { state.stack.push_back(state.params->resolve(param, state.stash)); } -void op_load_let(State &state, uint64_t param) { - state.stack.push_back(state.let_values[param]); -} - //----------------------------------------------------------------------------- void op_skip(State &state, uint64_t param) { @@ -69,17 +65,6 @@ void op_skip_if_false(State &state, uint64_t param) { //----------------------------------------------------------------------------- -void op_store_let(State &state, uint64_t) { - state.let_values.push_back(state.peek(0)); - state.stack.pop_back(); -} - -void op_evict_let(State &state, uint64_t) { - state.let_values.pop_back(); -} - -//----------------------------------------------------------------------------- - // compare lhs with a set member, short-circuit if found void op_check_member(State &state, uint64_t param) { if (state.peek(1).equal(state.peek(0))) { @@ -217,7 +202,7 @@ struct ProgramBuilder : public NodeVisitor, public NodeTraverser { bool is_typed_tensor_param(const Node &node) const { auto sym = as(node); - return (sym && (sym->id() >= 0) && is_typed_tensor(node)); + return (sym && is_typed_tensor(node)); } bool is_typed_tensor_product_of_params(const Node &node) const { @@ -262,12 +247,7 @@ struct ProgramBuilder : public NodeVisitor, public NodeTraverser { make_const_op(node, stash.create(node.value())); } void visit(const Symbol &node) override { - if (node.id() >= 0) { // param value - program.emplace_back(op_load_param, node.id()); - } else { // let binding - int let_offset = -(node.id() + 1); - program.emplace_back(op_load_let, let_offset); - } + program.emplace_back(op_load_param, node.id()); } void visit(const String &node) override { make_const_op(node, stash.create(node.hash())); @@ -292,12 +272,6 @@ struct ProgramBuilder : public NodeVisitor, public NodeTraverser { program[after_cond].update_param(after_true - after_cond); program[after_true].update_param(program.size() - after_true - 1); } - void visit(const Let &node) override { - node.value().traverse(*this); - program.emplace_back(op_store_let); - node.expr().traverse(*this); - program.emplace_back(op_evict_let); - } void visit(const Error &node) override { make_const_op(node, ErrorValue::instance); } @@ -498,7 +472,7 @@ struct ProgramBuilder : public NodeVisitor, public NodeTraverser { //------------------------------------------------------------------------- bool open(const Node &node) override { - if (check_type(node)) { + if (check_type(node)) { node.accept(*this); return false; } @@ -557,7 +531,6 @@ InterpretedFunction::State::State(const TensorEngine &engine_in) params(nullptr), stash(), stack(), - let_values(), program_offset(0) { } @@ -569,7 +542,6 @@ InterpretedFunction::State::init(const LazyParams ¶ms_in) { params = ¶ms_in; stash.clear(); stack.clear(); - let_values.clear(); program_offset = 0; if_cnt = 0; } diff --git a/eval/src/vespa/eval/eval/interpreted_function.h b/eval/src/vespa/eval/eval/interpreted_function.h index 140413727a3..8d20d8d5222 100644 --- a/eval/src/vespa/eval/eval/interpreted_function.h +++ b/eval/src/vespa/eval/eval/interpreted_function.h @@ -58,7 +58,6 @@ public: const LazyParams *params; Stash stash; std::vector stack; - std::vector let_values; uint32_t program_offset; uint32_t if_cnt; diff --git a/eval/src/vespa/eval/eval/key_gen.cpp b/eval/src/vespa/eval/eval/key_gen.cpp index 490f1c044ac..a745023fcf4 100644 --- a/eval/src/vespa/eval/eval/key_gen.cpp +++ b/eval/src/vespa/eval/eval/key_gen.cpp @@ -29,7 +29,6 @@ struct KeyGen : public NodeVisitor, public NodeTraverser { void visit(const Neg &) override { add_byte( 5); } void visit(const Not &) override { add_byte( 6); } void visit(const If &node) override { add_byte( 7); add_double(node.p_true()); } - void visit(const Let &) override { add_byte( 8); } void visit(const Error &) override { add_byte( 9); } void visit(const TensorMap &) override { add_byte(11); } // lambda should be part of key void visit(const TensorJoin &) override { add_byte(12); } // lambda should be part of key diff --git a/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp b/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp index 77e23b44799..303833e8e6c 100644 --- a/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp +++ b/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp @@ -74,7 +74,6 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser { llvm::IRBuilder<> builder; std::vector params; std::vector values; - std::vector let_values; llvm::Function *function; size_t num_params; PassParams pass_params; @@ -132,7 +131,6 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser { builder(context), params(), values(), - let_values(), function(nullptr), num_params(num_params_in), pass_params(pass_params_in), @@ -254,7 +252,7 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser { inside_forest = true; forest_end = &node; } - if (check_type(node)) { + if (check_type(node)) { node.accept(*this); return false; } @@ -352,13 +350,7 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser { push_double(item.value()); } void visit(const Symbol &item) override { - if (item.id() >= 0) { - push(get_param(item.id())); - } else { - int let_offset = -(item.id() + 1); - assert(size_t(let_offset) < let_values.size()); - push(let_values[let_offset]); - } + push(get_param(item.id())); } void visit(const String &item) override { push_double(item.hash()); @@ -402,13 +394,6 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser { phi->addIncoming(false_res, false_end); push(phi); } - void visit(const Let &item) override { - // NB: visit not open - item.value().traverse(*this); // NB: recursion - let_values.push_back(pop_double()); - item.expr().traverse(*this); // NB: recursion - let_values.pop_back(); - } void visit(const Error &) override { make_error(0); } diff --git a/eval/src/vespa/eval/eval/node_types.cpp b/eval/src/vespa/eval/eval/node_types.cpp index 3e942c7284e..d995c5281c4 100644 --- a/eval/src/vespa/eval/eval/node_types.cpp +++ b/eval/src/vespa/eval/eval/node_types.cpp @@ -14,22 +14,17 @@ class State private: const std::vector &_params; std::map &_type_map; - std::vector _let_types; std::vector _types; public: State(const std::vector ¶ms, std::map &type_map) - : _params(params), _type_map(type_map), _let_types(), _types() {} + : _params(params), _type_map(type_map), _types() {} const ValueType ¶m_type(size_t idx) { assert(idx < _params.size()); return _params[idx]; } - const ValueType &let_type(size_t idx) { - assert(idx < _let_types.size()); - return _let_types[idx]; - } const ValueType &peek(size_t ridx) const { assert(_types.size() > ridx); return _types[_types.size() - 1 - ridx]; @@ -43,31 +38,13 @@ public: _types.push_back(type); _type_map.emplace(&node, type); } - void push_let(const ValueType &type) { - _let_types.push_back(type); - } - void pop_let() { - assert(!_let_types.empty()); - _let_types.pop_back(); - } void assert_valid_end_state() const { - assert(_let_types.empty()); assert(_types.size() == 1); } }; -void action_bind_let(State &state) { - state.push_let(state.peek(0)); -} - -void action_unbind_let(State &state) { - state.pop_let(); -} - struct TypeResolver : public NodeVisitor, public NodeTraverser { State state; - using action_function = void (*)(State &); - std::vector> actions; TypeResolver(const std::vector ¶ms_in, std::map &type_map_out); ~TypeResolver(); @@ -75,21 +52,9 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { //------------------------------------------------------------------------- void assert_valid_end_state() const { - assert(actions.empty()); state.assert_valid_end_state(); } - void add_action(const Node &trigger, action_function action) { - actions.emplace_back(&trigger, action); - } - - void check_actions(const Node &node) { - if (!actions.empty() && (actions.back().first == &node)) { - actions.back().second(state); - actions.pop_back(); - } - } - //------------------------------------------------------------------------- void bind_type(const ValueType &type, const Node &node) { @@ -120,12 +85,7 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { bind_type(ValueType::double_type(), node); } void visit(const Symbol &node) override { - if (node.id() >= 0) { // param value - bind_type(state.param_type(node.id()), node); - } else { // let binding - int let_offset = -(node.id() + 1); - bind_type(state.let_type(let_offset), node); - } + bind_type(state.param_type(node.id()), node); } void visit(const String &node) override { bind_type(ValueType::double_type(), node); @@ -146,9 +106,6 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { bind_type(ValueType::any_type(), node); } } - void visit(const Let &node) override { - bind_type(state.peek(0), node); - } void visit(const Error &node) override { bind_type(ValueType::error_type(), node); } @@ -215,12 +172,7 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { //------------------------------------------------------------------------- - bool open(const Node &node) override { - auto let = as(node); - if (let) { - add_action(let->expr(), action_unbind_let); - add_action(let->value(), action_bind_let); - } + bool open(const Node &) override { return true; } @@ -228,17 +180,16 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { if (!check_error(node)) { node.accept(*this); } - check_actions(node); } }; TypeResolver::TypeResolver(const std::vector ¶ms_in, std::map &type_map_out) - : state(params_in, type_map_out), - actions() -{ } + : state(params_in, type_map_out) +{ +} -TypeResolver::~TypeResolver() { } +TypeResolver::~TypeResolver() {} } // namespace vespalib::eval::nodes:: } // namespace vespalib::eval::nodes diff --git a/eval/src/vespa/eval/eval/node_visitor.h b/eval/src/vespa/eval/eval/node_visitor.h index b12689e9603..91d65ceb7ec 100644 --- a/eval/src/vespa/eval/eval/node_visitor.h +++ b/eval/src/vespa/eval/eval/node_visitor.h @@ -26,7 +26,6 @@ struct NodeVisitor { virtual void visit(const nodes::Neg &) = 0; virtual void visit(const nodes::Not &) = 0; virtual void visit(const nodes::If &) = 0; - virtual void visit(const nodes::Let &) = 0; virtual void visit(const nodes::Error &) = 0; // tensor nodes @@ -97,7 +96,6 @@ struct EmptyNodeVisitor : NodeVisitor { void visit(const nodes::Neg &) override {} void visit(const nodes::Not &) override {} void visit(const nodes::If &) override {} - void visit(const nodes::Let &) override {} void visit(const nodes::Error &) override {} void visit(const nodes::TensorMap &) override {} void visit(const nodes::TensorJoin &) override {} diff --git a/eval/src/vespa/eval/eval/param_usage.cpp b/eval/src/vespa/eval/eval/param_usage.cpp index 717b8b6af8c..b43c0c01833 100644 --- a/eval/src/vespa/eval/eval/param_usage.cpp +++ b/eval/src/vespa/eval/eval/param_usage.cpp @@ -32,8 +32,7 @@ struct CountUsage : NodeTraverser { return true; } void close(const Node &node) override { - auto symbol = as(node); - if (symbol && (symbol->id() >= 0)) { + if (auto symbol = as(node)) { result[symbol->id()] += p; } } @@ -67,8 +66,7 @@ struct CheckUsage : NodeTraverser { return true; } void close(const Node &node) override { - auto symbol = as(node); - if (symbol && (symbol->id() >= 0)) { + if (auto symbol = as(node)) { result[symbol->id()] = 1.0; } } diff --git a/eval/src/vespa/eval/eval/test/eval_spec.cpp b/eval/src/vespa/eval/eval/test/eval_spec.cpp index 086e6570e0a..c7c0d754976 100644 --- a/eval/src/vespa/eval/eval/test/eval_spec.cpp +++ b/eval/src/vespa/eval/eval/test/eval_spec.cpp @@ -379,20 +379,6 @@ EvalSpec::add_if_cases() { [](double a){ if (a) { return 1.0; } else { return 0.0; } }); } -void -EvalSpec::add_let_cases() { - add_rule({"a", -10.0, 10.0}, "let(tmp,(a+1),(tmp*tmp))", [](double a){ return (a+1)*(a+1); }); - add_rule({"a", -10.0, 10.0}, "let(a,(a+1),((a*a)*a))", [](double a){ return (a+1)*(a+1)*(a+1); }); - add_rule({"a", -10.0, 10.0}, "let(a,(a+1),let(a,(a+1),let(b,2,let(a,(a+1),(a+b)))))", [](double a) { return (a + 5.0); }); - add_rule({"a", -10.0, 10.0}, {"b", -10.0, 10.0}, "let(a,(a*b),let(b,(b+a),(a*b)))", - [](double a, double b) - { - double let_a = (a * b); - double let_b = (b + let_a); - return (let_a * let_b); - }); -} - void EvalSpec::add_complex_cases() { add_expression({"a", "b"}, "((a<3)||b)") diff --git a/eval/src/vespa/eval/eval/test/eval_spec.h b/eval/src/vespa/eval/eval/test/eval_spec.h index 47f1424bd30..d942129c934 100644 --- a/eval/src/vespa/eval/eval/test/eval_spec.h +++ b/eval/src/vespa/eval/eval/test/eval_spec.h @@ -98,7 +98,6 @@ public: void add_set_membership_cases(); // a in [x, y, z] void add_boolean_cases(); // 1.0 && 0.0 void add_if_cases(); // if (a < b, a, b) - void add_let_cases(); // let (a, b + 1, a * a) void add_complex_cases(); // ... //------------------------------------------------------------------------- void add_all_cases() { @@ -110,7 +109,6 @@ public: add_set_membership_cases(); add_boolean_cases(); add_if_cases(); - add_let_cases(); add_complex_cases(); } //------------------------------------------------------------------------- diff --git a/eval/src/vespa/eval/eval/vm_forest.cpp b/eval/src/vespa/eval/eval/vm_forest.cpp index a9f310fd573..4a73394e354 100644 --- a/eval/src/vespa/eval/eval/vm_forest.cpp +++ b/eval/src/vespa/eval/eval/vm_forest.cpp @@ -111,7 +111,7 @@ void encode_less(const nodes::Less &less, { size_t meta_idx = model_out.size(); auto symbol = nodes::as(less.lhs()); - assert(symbol && (symbol->id() >= 0)); + assert(symbol); model_out.push_back(uint32_t(symbol->id()) << 12); assert(less.rhs().is_const()); encode_const(less.rhs().get_const_value(), model_out); @@ -129,7 +129,7 @@ void encode_in(const nodes::In &in, { size_t meta_idx = model_out.size(); auto symbol = nodes::as(in.lhs()); - assert(symbol && (symbol->id() >= 0)); + assert(symbol); model_out.push_back(uint32_t(symbol->id()) << 12); assert(in.rhs().is_const()); auto array = nodes::as(in.rhs()); -- cgit v1.2.3