Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

from_*(ptr, len) deprecation #2426

Closed
lrasinen opened this issue Oct 8, 2020 · 10 comments
Closed

from_*(ptr, len) deprecation #2426

lrasinen opened this issue Oct 8, 2020 · 10 comments
Assignees
Labels
documentation kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@lrasinen
Copy link

lrasinen commented Oct 8, 2020

From 3.9.1 release notes:

Passing iterator pairs or pointer/length pairs to parsing functions (basic_json::parse, basic_json::accept, basic_json::sax_parse, basic_json::from_cbor, basic_json::from_msgpack, basic_json::from_ubjson, basic_json::from_bson) via initializer lists is deprecated. Instead, pass the elements individually without braces.

However, the pointer/length overload is marked as deprecated at least for the from_* functions:

    JSON_HEDLEY_DEPRECATED_FOR(3.8.0, from_cbor(ptr, ptr + len))
    JSON_HEDLEY_WARN_UNUSED_RESULT
    static basic_json from_cbor(const T* ptr, std::size_t len, ...

Is this an error in the release notes or is the deprecation in error?

@nlohmann
Copy link
Owner

nlohmann commented Oct 8, 2020

You mean the version number?

@lrasinen
Copy link
Author

lrasinen commented Oct 8, 2020

My understanding of the release note wording is that e.g. from_cbor({ptr, len}) is deprecated, but that from_cbor(ptr, len) is the recommended alternative. But the second form is also deprecated.

@gregmarr
Copy link
Contributor

gregmarr commented Oct 8, 2020

The recommended alternative is not from_cbor(ptr, len) but from_cbor(ptr, ptr + len, ...).

@gregmarr
Copy link
Contributor

gregmarr commented Oct 8, 2020

@nlohmann Probably good to update the deprecation notice to include that one as well, as it's not really covered by the current note.

@lrasinen
Copy link
Author

lrasinen commented Oct 8, 2020

Agreed. My experience with this was something like this:

  • I got some binary data in ptr/len format
  • Went looking at the API whether that's supported directly
  • Found Doxygen docs, which do not show the deprecation status
  • Was baffled at compile warning
  • Went looking at the release notes which were unclear

I can (and do) use the ptr/ptr+len overload instead (although the ptr/len combo is quite convenient when applicable).

@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Dec 25, 2020
@nlohmann nlohmann added documentation and removed state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated labels Dec 26, 2020
@nlohmann
Copy link
Owner

Sorry for not reacting earlier. In order to clean the API after a lot of experimenting with how to cope with different input types, we decided that the overload that takes two iterators is the one we would like to proceed with.

The release notes are indeed misleading. They should read:

This release deprecates passing iterator pairs or pointer/length pairs to parsing functions (basic_json::parse, basic_json::accept, basic_json::sax_parse, basic_json::from_cbor, basic_json::from_msgpack, basic_json::from_ubjson, basic_json::from_bson) via initializer lists. Instead, pass two iterators; for instance, call basic_json::from_cbor(ptr, ptr+len) instead of basic_json::from_cbor({ptr, len}).

@lrasinen @gregmarr Do you agree with the new wording of the last sentence?

@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Dec 28, 2020
@lrasinen
Copy link
Author

I'm quite happy with suggested wording, thanks.

@nlohmann nlohmann self-assigned this Dec 29, 2020
@nlohmann
Copy link
Owner

I adjusted the release notes of version 3.8.0, 3.9.0, and 3.9.1.

@gregmarr
Copy link
Contributor

gregmarr commented Jan 4, 2021

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

3 participants