more explicit comma tracking

5 files changed, 39 insertions, 35 deletions

diff --git a/eval/src/vespa/eval/eval/basic_nodes.h b/eval/src/vespa/eval/eval/basic_nodes.h
index 01ac86e4472..52f39510a9b 100644
--- a/eval/src/vespa/eval/eval/basic_nodes.h
+++ b/eval/src/vespa/eval/eval/basic_nodes.h
@@ -157,7 +157,7 @@ public:
         str += " in [";
         CommaTracker node_list;
         for (const auto &node: _entries) {
-            node_list.maybe_comma(str);
+            node_list.maybe_add_comma(str);
             str += node->dump(ctx);
         }
         str += "])";
diff --git a/eval/src/vespa/eval/eval/function.cpp b/eval/src/vespa/eval/eval/function.cpp
index 8b49278a8f0..75d63ea7415 100644
--- a/eval/src/vespa/eval/eval/function.cpp
+++ b/eval/src/vespa/eval/eval/function.cpp
@@ -463,10 +463,9 @@ void parse_if(ParseContext &ctx) {
 }
 
 void parse_call(ParseContext &ctx, Call_UP call) {
+    CommaTracker list;
     for (size_t i = 0; i < call->num_params(); ++i) {
-        if (i > 0) {
-            ctx.eat(',');
-        }
+        list.maybe_eat_comma(ctx);
         call->bind_next(get_expression(ctx));
     }
     ctx.push_expression(std::move(call));
@@ -580,10 +579,9 @@ TensorSpec::Address get_tensor_address(ParseContext &ctx, const ValueType &type)
     TensorSpec::Address addr;
     ctx.skip_spaces();
     ctx.eat('{');
+    CommaTracker list;
     while (!ctx.find_list_end()) {
-        if (!addr.empty()) {
-            ctx.eat(',');
-        }
+        list.maybe_eat_comma(ctx);
         auto dim_name = get_ident(ctx, false);
         size_t dim_idx = type.dimension_index(dim_name);
         if (dim_idx != ValueType::Dimension::npos) {
@@ -617,10 +615,9 @@ void parse_tensor_create_verbose(ParseContext &ctx, const ValueType &type) {
     ctx.skip_spaces();
     ctx.eat('{');
     nodes::TensorCreate::Spec create_spec;
+    CommaTracker list;
     while (!ctx.find_list_end()) {
-        if (!create_spec.empty()) {
-            ctx.eat(',');
-        }
+        list.maybe_eat_comma(ctx);
         auto address = get_tensor_address(ctx, type);
         ctx.skip_spaces();
         ctx.eat(':');
@@ -638,20 +635,18 @@ void parse_tensor_create_convenient(ParseContext &ctx, const ValueType &type,
     nodes::TensorCreate::Spec create_spec;
     using Label = TensorSpec::Label;
     std::vector<Label> addr;
+    std::vector<CommaTracker> list;
     for (;;) {
         if (addr.size() == dim_list.size()) {
             TensorSpec::Address address;
             for (size_t i = 0; i < addr.size(); ++i) {
-                if (addr[i].is_mapped()) {
-                    address.emplace(dim_list[i].name, addr[i]);
-                } else {
-                    address.emplace(dim_list[i].name, Label(addr[i].index-1));
-                }
+                address.emplace(dim_list[i].name, addr[i]);
             }
             create_spec.emplace(std::move(address), get_expression(ctx));
         } else {
             bool mapped = dim_list[addr.size()].is_mapped();
             addr.push_back(mapped ? Label("") : Label(size_t(0)));
+            list.emplace_back();
             ctx.skip_spaces();
             ctx.eat(mapped ? '{' : '[');
         }
@@ -659,25 +654,23 @@ void parse_tensor_create_convenient(ParseContext &ctx, const ValueType &type,
             bool mapped = addr.back().is_mapped();
             ctx.eat(mapped ? '}' : ']');
             addr.pop_back();
+            list.pop_back();
             if (addr.empty()) {
                 return ctx.push_expression(std::make_unique<nodes::TensorCreate>(type, std::move(create_spec)));
             }
         }
-        if (addr.back().is_mapped()) {
-            if (addr.back().name != "") {
-                ctx.eat(',');
+        if (list.back().maybe_eat_comma(ctx)) {
+            if (addr.back().is_indexed()) {
+                if (++addr.back().index >= dim_list[addr.size()-1].size) {
+                    return ctx.fail(make_string("dimension too large: '%s'",
+                                    dim_list[addr.size()-1].name.c_str()));
+                }
             }
+        }
+        if (addr.back().is_mapped()) {
             addr.back().name = get_ident(ctx, false);
             ctx.skip_spaces();
             ctx.eat(':');
-        } else {
-            if (addr.back().index != 0) {
-                ctx.eat(',');
-            }
-            if (++addr.back().index > dim_list[addr.size()-1].size) {
-                return ctx.fail(make_string("dimension too large: '%s'",
-                                            dim_list[addr.size()-1].name.c_str()));
-            }
         }
     }
 }
@@ -811,11 +804,9 @@ void parse_in(ParseContext &ctx)
     ctx.skip_spaces();
     ctx.eat('[');
     ctx.skip_spaces();
-    size_t size = 0;
-    while (!ctx.eos() && ctx.get() != ']') {
-        if (++size > 1) {
-            ctx.eat(',');
-        }
+    CommaTracker list;
+    while (!ctx.find_list_end()) {
+        list.maybe_eat_comma(ctx);
         parse_value(ctx);
         ctx.skip_spaces();
         auto entry = ctx.pop_expression();
diff --git a/eval/src/vespa/eval/eval/string_stuff.cpp b/eval/src/vespa/eval/eval/string_stuff.cpp
index 70331a38cca..782aacb5cea 100644
--- a/eval/src/vespa/eval/eval/string_stuff.cpp
+++ b/eval/src/vespa/eval/eval/string_stuff.cpp
@@ -9,7 +9,7 @@ vespalib::string as_string(const TensorSpec::Address &address) {
     CommaTracker label_list;
     vespalib::string str = "{";
     for (const auto &label: address) {
-        label_list.maybe_comma(str);
+        label_list.maybe_add_comma(str);
         if (label.second.is_mapped()) {
             str += make_string("%s:%s", label.first.c_str(), label.second.name.c_str());
         } else {
diff --git a/eval/src/vespa/eval/eval/string_stuff.h b/eval/src/vespa/eval/eval/string_stuff.h
index 2e60691fa28..3cbc7c9d35b 100644
--- a/eval/src/vespa/eval/eval/string_stuff.h
+++ b/eval/src/vespa/eval/eval/string_stuff.h
@@ -9,16 +9,29 @@ namespace vespalib::eval {
 
 /**
  * Helper class used to insert commas on the appropriate places in
- * comma-separated textual lists.
+ * comma-separated textual lists. Can also be used to figure out when
+ * to expect commas when parsing text.
  **/
 struct CommaTracker {
     bool first;
     CommaTracker() : first(true) {}
-    void maybe_comma(vespalib::string &dst) {
+    bool maybe_add_comma(vespalib::string &dst) {
         if (first) {
             first = false;
+            return false;
         } else {
             dst.push_back(',');
+            return true;
+        }
+    }
+    template <typename T>
+    bool maybe_eat_comma(T &ctx) {
+        if (first) {
+            first = false;
+            return false;
+        } else {
+            ctx.eat(',');
+            return true;
         }
     }
 };
diff --git a/eval/src/vespa/eval/eval/tensor_nodes.h b/eval/src/vespa/eval/eval/tensor_nodes.h
index 776275f5884..93c1da4f2a8 100644
--- a/eval/src/vespa/eval/eval/tensor_nodes.h
+++ b/eval/src/vespa/eval/eval/tensor_nodes.h
@@ -234,7 +234,7 @@ public:
         str += ":{";
         CommaTracker child_list;
         for (const Child &child: _cells) {
-            child_list.maybe_comma(str);
+            child_list.maybe_add_comma(str);
             str += as_string(child.first);
             str += ":";
             str += child.second->dump(ctx);