Parsed bytes offset not updated when visitor returns false

[rcjmurillo]

Hi, is there any particular reason of why the parsed bytes offset is not updated when the returned code is not PARSE_SUCCESS or PARSE_CONTINUE? That's the case when the visitor returns false from any of it's methods, I'm needing this for a particular case but I think it could also be pretty useful to know the offset if the visitor stops the parse process.

msgpack-c/include/msgpack/v2/parse.hpp

Lines 1023 to 1049 in b02c6be

	template <typename Visitor>
	inline parse_return
	parse_imp(const char* data, size_t len, size_t& off, Visitor& v) {
	std::size_t noff = off;
	if(len <= noff) {
	// FIXME
	v.insufficient_bytes(noff, noff);
	return PARSE_CONTINUE;
	}
	detail::parse_helper<Visitor> h(v);
	parse_return ret = h.execute(data, len, noff);
	switch (ret) {
	case PARSE_CONTINUE:
	off = noff;
	v.insufficient_bytes(noff - 1, noff);
	return ret;
	case PARSE_SUCCESS:
	off = noff;
	if(noff < len) {
	return PARSE_EXTRA_BYTES;
	}
	return ret;
	default:
	return ret;
	}
	}

[redboltz]

Hi @rcjmurillo ,

Hi, is there any particular reason of why the parsed bytes offset is not updated when the returned code is not PARSE_SUCCESS or PARSE_CONTINUE?

It's simply "if an error happens, do not update" policy.

That's the case when the visitor returns false from any of it's methods, I'm needing this for a particular case but I think it could also be pretty useful to know the offset if the visitor stops the parse process.

Do you mean user can get the information where the error is occurred ?
I think so too.

Let me consider the original behavior.

If offset is NOT updated, and if the user get an error, the user doesn't have much options. At most one byte advance and retry :<
If the user don't want to update the offset, the user can make a copy of the offset before calling parse.

Hence, I think that always updating the offset is acceptable breaking change.

[redboltz]

@rcjmurillo , I've just implemented the feature.
Could you try #639?

Here is the test cases corresponding to visitor returns false:
https://github.com/msgpack/msgpack-c/pull/639/files#diff-9d56f20379e66ee4d732ea417b491431R143

[redboltz]

@rcjmurillo , does #639 work as you expected?

[redboltz]

merged.