Re: [tarantool-patches] Re: [PATCH v1 1/1] Implement mp_stack_top for mp_stack class

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

> Now it's not that bad, don't you agree? We even managed to get rid of
> the empty array/map check.

Yep. Looks good. On the branch.

====================================================

Introduced a new mp_stack_top method for mp_stack class to
return the pointer to a top frame of the stack.

Class methods have been redesigned so that this new mp_stack_top
helper fits into the API naturally.

This is required in scope of multikey indexes to keep a pointer
to a multikey frame and extract currently processed item index
of this frame later.

Needed for https://github.com/tarantool/tarantool/issues/1257
---
 msgpuck.c | 16 ++++++------
 msgpuck.h | 76 ++++++++++++++++++++-----------------------------------
 2 files changed, 36 insertions(+), 56 deletions(-)

diff --git a/msgpuck.c b/msgpuck.c
index 05e472e..d0ffb83 100644
--- a/msgpuck.c
+++ b/msgpuck.c
@@ -306,16 +306,16 @@ next:										\
 		return -1;							\
 	}									\
 	while (!mp_stack_is_empty(&stack)) {					\
-		enum mp_type type = mp_stack_type(&stack);			\
-		int curr = mp_stack_advance(&stack);				\
-		if (curr == 0 || mp_stack_count(&stack) == 0)			\
-			PRINTF(type == MP_ARRAY ? "[" : "{");			\
-		if (curr == -1) {						\
-			PRINTF(type == MP_ARRAY ? "]" : "}");			\
+		struct mp_frame *frame = mp_stack_top(&stack);			\
+		if (frame->idx < 0)						\
+			PRINTF(frame->type == MP_ARRAY ? "[" : "{");		\
+		if (!mp_frame_advance(frame)) {					\
+			PRINTF(frame->type == MP_ARRAY ? "]" : "}");		\
 			mp_stack_pop(&stack);					\
 			continue;						\
-		} else if (curr != 0) {						\
-			PRINTF(type == MP_MAP && curr % 2 == 1 ? ": " : ", ");	\
+		} else if (frame->idx != 0) {					\
+			PRINTF(frame->type == MP_MAP && frame->idx % 2 == 1 ?	\
+			       ": " : ", ");					\
 		}								\
 		goto next;							\
 	}									\
diff --git a/msgpuck.h b/msgpuck.h
index eab1339..64b10e6 100644
--- a/msgpuck.h
+++ b/msgpuck.h
@@ -1222,7 +1222,7 @@ struct mp_frame {
 	 * Index of currently processing item. Must be less than
 	 * mp_frame::count member.
 	 */
-	int curr;
+	int idx;
 };
 
 /**
@@ -1278,6 +1278,14 @@ mp_stack_is_empty(struct mp_stack *stack);
 MP_PROTO bool
 mp_stack_is_full(struct mp_stack *stack);
 
+/**
+ * \brief Return the top mp_stack \a stack frame.
+ * \param stack - the pointer to a mp_stack to operate with.
+ * \pre mp_stack_is_empty(stack) == false
+ */
+MP_PROTO struct mp_frame *
+mp_stack_top(struct mp_stack *stack);
+
 /**
  * \brief Pop the top mp_stack \a stack frame.
  * \param stack - the pointer to a mp_stack to operate with.
@@ -1298,33 +1306,13 @@ MP_PROTO void
 mp_stack_push(struct mp_stack *stack, enum mp_type type, int count);
 
 /**
- * \brief Get type attribute of the \a stack top frame.
- * \param stack - the pointer to a stack to operate with.
- * \retval enum mp_type value - the top stack frame type.
- * \pre mp_stack_is_empty(stack) == false
- */
-MP_PROTO enum mp_type
-mp_stack_type(struct mp_stack *stack);
-
-/**
- * \brief Get count attribute of the \a stack top frame.
- * \param stack - the pointer to a stack to operate with.
- * \retval count - the top stack frame items count.
- * \pre mp_stack_is_empty(stack) == false
- */
-MP_PROTO int
-mp_stack_count(struct mp_stack *stack);
-
-/**
- * \brief Advance curr attribute of the \a stack top frame.
- * \param stack - the pointer to a stack to operate with.
- * \retval index of the element to process if the top
- *         mp_frame::curr is less than top mp_frame::count field.
- *         -1 otherwise.
- * \pre mp_stack_is_empty(stack) == false
+ * \brief Advance idx attribute of the \a frame.
+ * \param frame - the frame pointer to operate with.
+ * \retval true when mp_frame::idx is less than  mp_frame::count.
+ *         false otherwise.
  */
-MP_PROTO int
-mp_stack_advance(struct mp_stack *stack);
+MP_PROTO bool
+mp_frame_advance(struct mp_frame *frame);
 
 /*
  * }}}
@@ -2349,6 +2337,12 @@ mp_stack_is_full(struct mp_stack *stack)
 	return stack->used >= stack->size;
 }
 
+MP_IMPL struct mp_frame *
+mp_stack_top(struct mp_stack *stack)
+{
+	return &stack->frames[stack->used - 1];
+}
+
 MP_IMPL void
 mp_stack_pop(struct mp_stack *stack)
 {
@@ -2363,30 +2357,16 @@ mp_stack_push(struct mp_stack *stack, enum mp_type type, int count)
 	int idx = stack->used++;
 	stack->frames[idx].type = type;
 	stack->frames[idx].count = count;
-	stack->frames[idx].curr = -1;
-}
-
-MP_IMPL enum mp_type
-mp_stack_type(struct mp_stack *stack)
-{
-	assert(!mp_stack_is_empty(stack));
-	return stack->frames[stack->used - 1].type;
+	stack->frames[idx].idx = -1;
 }
 
-MP_IMPL int
-mp_stack_count(struct mp_stack *stack)
-{
-	return stack->frames[stack->used - 1].count;
-}
-
-MP_IMPL int
-mp_stack_advance(struct mp_stack *stack)
+MP_IMPL bool
+mp_frame_advance(struct mp_frame *frame)
 {
-	assert(!mp_stack_is_empty(stack));
-	struct mp_frame *frame = &stack->frames[stack->used - 1];
-	if (frame->curr < frame->count - 1)
-		return ++frame->curr;
-	return -1;
+	if (frame->idx >= frame->count - 1)
+		return false;
+	++frame->idx;
+	return true;
 }
 
 /** \endcond */
-- 
2.21.0