diff options
author | Håvard Pettersen <havardpe@oath.com> | 2020-03-18 15:33:19 +0000 |
---|---|---|
committer | Håvard Pettersen <havardpe@oath.com> | 2020-03-19 10:49:32 +0000 |
commit | 56abaa52add9945179344c7289b896b790315388 (patch) | |
tree | eef62abe410e9d0584483a7a00c916ac35c0b26a /eval | |
parent | 141c98cbcf97153858d6ae5a535e331d7cf174ca (diff) |
print more details about type errors
Diffstat (limited to 'eval')
-rw-r--r-- | eval/src/tests/eval/node_types/node_types_test.cpp | 12 | ||||
-rw-r--r-- | eval/src/vespa/eval/eval/node_types.cpp | 95 | ||||
-rw-r--r-- | eval/src/vespa/eval/eval/node_types.h | 3 |
3 files changed, 89 insertions, 21 deletions
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 07bca5b53a6..900e71aa899 100644 --- a/eval/src/tests/eval/node_types/node_types_test.cpp +++ b/eval/src/tests/eval/node_types/node_types_test.cpp @@ -29,6 +29,11 @@ void verify(const vespalib::string &type_expr, const vespalib::string &type_spec input_types.push_back(ValueType::from_spec(function->param_name(i))); } NodeTypes types(*function, input_types); + if (!types.errors().empty()) { + for (const auto &msg: types.errors()) { + fprintf(stderr, "type error: %s\n", msg.c_str()); + } + } ValueType expected_type = ValueType::from_spec(type_spec); ValueType actual_type = types.get_type(function->root()); EXPECT_EQUAL(expected_type, actual_type); @@ -236,6 +241,13 @@ TEST("require that tensor create resolves correct type") { TEST_DO(verify("tensor(x[3]):{{x:0}:double,{x:1}:error,{x:2}:double}", "error")); } +TEST("require that tensor lambda resolves correct type") { + TEST_DO(verify("tensor(x[3])(error)", "error")); + TEST_DO(verify("tensor(x[3])(double)", "tensor(x[3])")); + TEST_DO(verify("tensor<float>(x[3])(double)", "tensor<float>(x[3])")); + TEST_DO(verify("tensor(x[3])(tensor(x[2]))", "error")); +} + TEST("require that tensor peek resolves correct type") { TEST_DO(verify("tensor(x[3]){x:1}", "double")); TEST_DO(verify("tensor(x[3]){x:double}", "error")); diff --git a/eval/src/vespa/eval/eval/node_types.cpp b/eval/src/vespa/eval/eval/node_types.cpp index 37632e35693..afb51b5b605 100644 --- a/eval/src/vespa/eval/eval/node_types.cpp +++ b/eval/src/vespa/eval/eval/node_types.cpp @@ -3,6 +3,10 @@ #include "check_type.h" #include "node_traverser.h" #include "node_types.h" +#include <vespa/vespalib/util/stringfmt.h> +#include <vespa/vespalib/util/classname.h> + +using vespalib::make_string_short::fmt; namespace vespalib::eval { namespace nodes { @@ -13,11 +17,13 @@ class State private: const std::vector<ValueType> &_params; std::map<const Node *, ValueType> &_type_map; + std::vector<vespalib::string> &_errors; public: State(const std::vector<ValueType> ¶ms, - std::map<const Node *, ValueType> &type_map) - : _params(params), _type_map(type_map) {} + std::map<const Node *, ValueType> &type_map, + std::vector<vespalib::string> &errors) + : _params(params), _type_map(type_map), _errors(errors) {} const ValueType ¶m_type(size_t idx) { assert(idx < _params.size()); @@ -33,20 +39,44 @@ public: assert(pos != _type_map.end()); return pos->second; } + void add_error(const vespalib::string &msg) { + _errors.push_back(msg); + } }; struct TypeResolver : public NodeVisitor, public NodeTraverser { State state; TypeResolver(const std::vector<ValueType> ¶ms_in, - std::map<const Node *, ValueType> &type_map_out); + std::map<const Node *, ValueType> &type_map_out, + std::vector<vespalib::string> &errors_out); ~TypeResolver(); const ValueType ¶m_type(size_t idx) { return state.param_type(idx); } - void bind(const ValueType &type, const Node &node) { - state.bind(type, node); + void fail(const Node &node, const vespalib::string &msg, bool child_types = true) { + auto str = fmt("%s: %s", vespalib::getClassName(node).c_str(), msg.c_str()); + if (child_types) { + str += ", child types: ["; + for (size_t i = 0; i < node.num_children(); ++i) { + if (i > 0) { + str += ", "; + } + str += state.type(node.get_child(i)).to_spec(); + } + str += "]"; + } + state.add_error(str); + state.bind(ValueType::error_type(), node); + } + + void bind(const ValueType &type, const Node &node, bool check_error = true) { + if (check_error && type.is_error()) { + fail(node, "type resolving failed"); + } else { + state.bind(type, node); + } } const ValueType &type(const Node &node) { @@ -65,7 +95,7 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { bool check_error(const Node &node) { for (size_t i = 0; i < node.num_children(); ++i) { if (type(node.get_child(i)).is_error()) { - bind(ValueType::error_type(), node); + bind(ValueType::error_type(), node, false); return true; } } @@ -87,7 +117,7 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { bind(ValueType::double_type(), node); } void visit(const Symbol &node) override { - bind(param_type(node.id()), node); + bind(param_type(node.id()), node, false); } void visit(const String &node) override { bind(ValueType::double_type(), node); @@ -100,7 +130,7 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { type(node.false_expr())), node); } void visit(const Error &node) override { - bind(ValueType::error_type(), node); + bind(ValueType::error_type(), node, false); } void visit(const TensorMap &node) override { resolve_op1(node); } void visit(const TensorJoin &node) override { resolve_op2(node); } @@ -109,12 +139,30 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { type(node.get_child(1))), node); } void visit(const TensorReduce &node) override { - const ValueType &child = type(node.get_child(0)); - bind(child.reduce(node.dimensions()), node); + auto my_type = type(node.get_child(0)).reduce(node.dimensions()); + if (my_type.is_error()) { + auto str = fmt("aggr: %s, dimensions: [", + AggrNames::name_of(node.aggr())->c_str()); + for (const auto &dimension: node.dimensions()) { + str += ","; + str += dimension; + } + str += "]"; + fail(node, str); + } else { + bind(my_type, node); + } } void visit(const TensorRename &node) override { - const ValueType &child = type(node.get_child(0)); - bind(child.rename(node.from(), node.to()), node); + auto my_type = type(node.get_child(0)).rename(node.from(), node.to()); + if (my_type.is_error()) { + auto str = fmt("%s -> %s", + TensorRename::flatten(node.from()).c_str(), + TensorRename::flatten(node.to()).c_str()); + fail(node, str); + } else { + bind(my_type, node); + } } void visit(const TensorConcat &node) override { bind(ValueType::concat(type(node.get_child(0)), @@ -123,7 +171,7 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { void visit(const TensorCreate &node) override { for (size_t i = 0; i < node.num_children(); ++i) { if (!type(node.get_child(i)).is_double()) { - return bind(ValueType::error_type(), node); + return fail(node, fmt("non-double child at index %zu", i), false); } } bind(node.type(), node); @@ -139,7 +187,7 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { } NodeTypes lambda_types(node.lambda(), arg_types); if (!lambda_types.get_type(node.lambda().root()).is_double()) { - return bind(ValueType::error_type(), node); + return fail(node, "lambda function produces non-double result", false); } import(lambda_types); bind(node.type(), node); @@ -151,20 +199,22 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { dimensions.push_back(dim.first); if (dim.second.is_expr()) { if (!type(*dim.second.expr).is_double()) { - return bind(ValueType::error_type(), node); + return fail(node, fmt("non-double label expression for dimension %s", dim.first.c_str())); } } else { size_t dim_idx = param_type.dimension_index(dim.first); if (dim_idx == ValueType::Dimension::npos) { - return bind(ValueType::error_type(), node); + return fail(node, fmt("dimension not in param: %s", dim.first.c_str())); } const auto ¶m_dim = param_type.dimensions()[dim_idx]; if (param_dim.is_indexed()) { if (!is_number(dim.second.label)) { - return bind(ValueType::error_type(), node); + return fail(node, fmt("non-numeric label for dimension %s: '%s'", + dim.first.c_str(), dim.second.label.c_str())); } if (as_number(dim.second.label) >= param_dim.size) { - return bind(ValueType::error_type(), node); + return fail(node, fmt("out-of-bounds label for dimension %s: %s", + dim.first.c_str(), dim.second.label.c_str())); } } } @@ -227,8 +277,9 @@ struct TypeResolver : public NodeVisitor, public NodeTraverser { }; TypeResolver::TypeResolver(const std::vector<ValueType> ¶ms_in, - std::map<const Node *, ValueType> &type_map_out) - : state(params_in, type_map_out) + std::map<const Node *, ValueType> &type_map_out, + std::vector<vespalib::string> &errors_out) + : state(params_in, type_map_out, errors_out) { } @@ -248,10 +299,12 @@ NodeTypes::NodeTypes(const Function &function, const std::vector<ValueType> &inp _type_map() { assert(input_types.size() == function.num_params()); - nodes::TypeResolver resolver(input_types, _type_map); + nodes::TypeResolver resolver(input_types, _type_map, _errors); function.root().traverse(resolver); } +NodeTypes::~NodeTypes() = default; + const ValueType & NodeTypes::get_type(const nodes::Node &node) const { diff --git a/eval/src/vespa/eval/eval/node_types.h b/eval/src/vespa/eval/eval/node_types.h index a93f886a2eb..c072915ffb1 100644 --- a/eval/src/vespa/eval/eval/node_types.h +++ b/eval/src/vespa/eval/eval/node_types.h @@ -23,9 +23,12 @@ class NodeTypes private: ValueType _not_found; std::map<const nodes::Node*,ValueType> _type_map; + std::vector<vespalib::string> _errors; public: NodeTypes(); NodeTypes(const Function &function, const std::vector<ValueType> &input_types); + ~NodeTypes(); + const std::vector<vespalib::string> &errors() const { return _errors; } const ValueType &get_type(const nodes::Node &node) const; template <typename F> void each(F &&f) const { |