Binary subtype field cannot represent all CBOR tags

[DavidJRobertson]

The subtype field on binary_t/byte_container_with_subtype has type uint8_t. Since these subtypes are mapped to CBOR tags, I had expected to be able to use tag values above 255 (list of tags: https://www.iana.org/assignments/cbor-tags/cbor-tags.xhtml)

[nlohmann]

Indeed the library currently only supports subtypes in the range of 0..255. I can't remember why we decided for std::uint8_t when we introduced subtypes - we definitely were not aware of https://www.iana.org/assignments/cbor-tags/cbor-tags.xhtml. I'm afraid changing the subtype value to a bigger integer would be a breaking change - or would it?

[gregmarr]

It would change the size of the object and the return type of this function (which has an error in its comment, it doesn't return -1 for unset):

/*!
@brief return the binary subtype

Returns the numerical subtype of the value if it has a subtype. If it does
not have a subtype, this function will return size_t(-1) as a sentinel
value.

@return the numerical subtype of the binary value
*/
constexpr std::uint8_t subtype() const noexcept
{
    return m_subtype;
}

This could potentially cause build errors if it were changed if someone assigns it to a uint8_t and there's now a narrowing conversion. It could also cause issues if two different modules were compiled with different versions and passed objects around, but I'm not sure that's something that should be supported anyway.

[nlohmann]

Good catch noticing the issue with the return type!

[DavidJRobertson]

Also, is it possible to add a binary element without tagging it at all? Tag number 0 cannot be used for this purpose as it has been assigned a meaning.

[nlohmann]

The binary type has a member function has_subclass to check this. See https://json.nlohmann.me/features/binary_values/#api-for-binary-values

[nlohmann]

I started to work on the issue, see #2908. Next step would be to set subtype_type to std::uint64_t and check the binary readers/writers if std::uint8_t is hardcoded somewhere.

[gregmarr]

Does the default value of the subtype member need to change from 0 to -1 to match the comment, or does the comment need to be updated?

[nlohmann]

The value of m_subtype is only returned by subtype() if m_has_subtype is true. It is initially false, and can only be set to true with set_subtype or one of the constructors taking a subtype. I think the default value is never returned.

[nlohmann]

Next steps on #2908: the binary subtype uses std::uint64_t now. As consequent next step I would check if cbor_tag_handler_t can actually be extended to also read binary values with subtypes.

[nlohmann]

@DavidJRobertson @gregmarr PR #2908 is now ready for review. All that is missing is overworking the documentation wrt. the new subtype type and the fact that tags in CBOR can now be stored (rather than just being ignore).

[gregmarr]

@nlohmann Ah, you're right, I missed that fix in the function itself.

[nlohmann]

I would merge the branch later today. Feedback still welcome!