Moving forward to version 3.0.0

[nlohmann]

I kept postponing the step toward version 3.0.0 for quite some time and quite some issues have been piled up. These issues have in common that fixing them will break the public API - be it very obvious such as removing deprecated functions or changing the type of exceptions or rather subtle such as begin able to store a NaN value.

Question is: How to proceed? So far, I worked in the develop branch. As soon as I, for instance, remove deprecated functions and push to develop, this code may break existing applications. I can live with that - we still have master as a branch containing the code of the last release, yesterday's 2.1.1. Furthermore, OSS-Fuzz for instance, checks the develop branch, so it would be a bit dangerous to only work in a feature branch.

I wonder whether it would be bad style to continue working in develop. What do you think?

[ibroheem]

"I wonder whether it would be bad style to continue working in develop"
Thats why its called develop/experimental etc...

How about doing it the GCC way:

4.9.x
5.x.x
6.x.x

Everything in parallel:
2.x.x
3.x.x

Just my own opinion.

[theodelrieu]

That's a good idea, we should separate branches, since we will surely backports bug fixes from 3.0.0 to previous versions.

GCC still releases 4.9.x

[nlohmann]

(We have to keep in mind that the GCC project has far more people, and I could live without 2.x.x releases while preparing for 3.0.0.)

[ibroheem]

2.x.x: No new feature additions, Only fixes. Define an LTS, few months or years.
3.x.x: New features, break the breakable if possible.

[gregmarr]

If you need a definition for what it means to have 3.0.0 vs 2.x.x, I suggest you go with semver.org's definition:

Summary

Given a version number MAJOR.MINOR.PATCH, increment the:

MAJOR version when you make incompatible API changes,
MINOR version when you add functionality in a backwards-compatible manner, and
PATCH version when you make backwards-compatible bug fixes.
Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format.

As for where you do it, I wouldn't expect anyone to be basing production code on a develop branch, so changing that to 3.0.0-alpha and keeping master as 2.x.x until 3.0.0 is ready for master should be fine. If any additional fixes are needed in 2.x.x until that time, a temporary branch could be created for that.

[nlohmann]

@ibroheem I don't think I have the manpower to support 2.x.x for too long...

@gregmarr I like semver - I was just unsure about the actual process and how to implement it with Github. I totally agree that no-one should work trust develop blindly. I just wanted to make sure I'm not ignoring best practices.

[TurpentineDistillery]

I don't think I have the manpower to support 2.x.x for too long...

Even if you expect minimal legacy support for 2.x.x., the branching model should still allow for it, in case you need to make a trivial but important fix now and then.

[nlohmann]

I added a wiki page with notes on the road toward version 3.0.0. I also linked this page in the README file. You all have been warned ;-)

[theodelrieu]

Hi, here are some things I had in mind for 3.0.0:

Reduce the number of basic_json template arguments

There is just way too many, the symbols are hard to read, and can crash compilers (especially MSVC) if they become way too complex, (e.g. I had to refrain using detail::disjunction and detail::conjunction).

An easy solution is a single template argument JsonTraits, with all the existing arguments defined inside. (Just like std::iterator_traits)

Right now, it's (almost) impossible to template on basic_json<..., ...>, you need at least 15 typename before writing down the return type, whereas it could simply be:

template <typename Traits, typename T>
void to_json(basic_json<Traits>&, const T&);

Bonus: Forward-declaration becomes really easy, AND it doesn't include map, vector, etc:

namespace nlohmann {
struct json_traits;

template <typename Traits> class basic_json;
using json = basic_json<json_traits>;
}

I could be mistaken, not being an expert with this, but I think updating json_traits won't break anything (from an ABI point-of-view, please correct me if I'm wrong).

We should look at taocpp, I remember one of the maintainers mentioning they were using this technique, during the premises of to/from_json

Split the library into multiple headers

This is mainly a matter of clarity, it's really hard to read through the class (which is at ~12K lines right now).
It could also stop possible contributors from jumping in the project.

However I know we want to keep releasing the single json.hpp file for people who wants it.
Looking at what Catch does on that matter would be great.

Each basic_json nested struct/class could go into a separate header, and most private methods could land into the detail namespace.

It could look like this (I didn't think about the details like friendship etc):

namespace nlohmann {
namespace detail {
 struct json_pointer {};
}
class basic_json {
public:
using json_pointer = detail::json_pointer;
};
}

About exceptions

I haven't put a lot of thinking into exceptions, but wouldn't it make sense if the only exceptions thrown by the library were derived from json::exception?
We could also encapsulate user-defined exceptions with nested exceptions? (I've never tried to use those, but they sound appropriate for that matter)

I'd like to have access to Boost.Outcome or std::expected to satisfy users who disable exceptions, unfortunately that's not possible!

(Minor thing): clang-format

@nlohmann I know you like astyle, but I forget about it everytime! Could you find the time to configure a small clang-format, that approaches your code-beauty standards?
If not, that's not a major problem ;)

Well, this post was longer than expected.

[jaredgrubb]

+1 on changing the template to a traits/policy class. I did file an issue on this because I think there's some really cool things you could do with that (#456).

[nlohmann]

@theodelrieu I am currently working at the exceptions. I plan to have a class json::exception with several derived classes like json::parse_error.

[nlohmann]

I shall have a look at clang-format, but it seems to be a lot of work to make it work like astyle.

[theodelrieu]

Hi, I've been thinking about something a bit crazy the past few days!

One of the problems I also have with basic_json is that most of its public static functions could be in a namespace (i.e. most of the code doesn't belong here).

I'd like to keep only constructors, query methods, operator[] and such (i.e relevant parts of a JSON value).

DISCLAIMER
This might imply a lot of refactoring, yet I believe this is really feasible, and should be done in a major release.

What I have in mind:

namespace njson {
// this struct exposes each template arg (just like basic_json does: object_t, etc...)
template <...> struct basic_json_traits{/*...*/};
using default_traits = basic_json_traits<>;

template <typename JSONTraits = default_traits>
class basic_value {
public:
  // for 2.x.x compat
  static basic_value parse(...) [[deprecated]] {
    return ::njson::parse<basic_value>(...);
  }

  // and so on for the other methods
};
using value = basic_value<>;

// default traits: auto val = njson::parse(...);
// custom traits: auto val = njson::parse<my_custom_json_value>(...);
template <typename BasicJsonValue = value>
BasicJsonValue parse(...) {/*...*/}
} // namespace njson

// old code will still compile with this
namespace nlohmann [[deprecated]] {
  template <typename T, typename SFINAE>
  using adl_serializer = ::njson::adl_serializer<T, SFINAE>;

  template <...>
  using basic_json = ::njson::basic_value<::njson::basic_json_traits<...>>;
  
  using json = ::njson::value;
}

There is a slight change on each method call for custom json values, (e.g. parse in this example).
I did not rename the namespace to json, it would break code where using nlohmann::json; is used.

I can start a POC on my fork, I'm quite confident that everything should work as expected.

What do you all think about this? I'm waiting for your inputs :)

PS:
Now that I think of it, with this change it would be possible to make a static/shared library in the future, with all the internals, and non-template methods we use.
I remember some users signaling they wanted this to be possible. But that's really looking far ahead...

[nlohmann]

@theodelrieu We should discuss this in Slack some time.

[theodelrieu]

Sure, I will post on slack at what time I'm available this weekend

[gregmarr]

Is there a slack team/channel for this project?

[theodelrieu]

Here it is, however I think I need to invite your email address.
You can send me a mail and I'll do that quickly!

We might set up an IRC channel at freenode, it would be easier for everybody to join, ask questions, and participate to discussions.

[nlohmann]

Let's please use Slack - opening a second channel would just mean more work...

[theodelrieu]

That's right, my bad

[nlohmann]

This may be on short notice, but I am in Slack (https://nlohmannjson.slack.com/messages/general) now and most likely for the next 3 hours. @theodelrieu @gregmarr and everybody, feel free to join!

[nlohmann]

@theodelrieu I thought about your proposal to reorganize the classes; that is, move most of them out of basic_json into the detail namespace and to re-introduce them to basic_json with using.

Here is the current structure:

• namespace nlohmann
  • namespace detail
    • class exception
    • class parse_error
    • class invalid_iterator
    • class type_error
    • class out_of_range
    • class other_error
  • class basic_json
    • class serializer (private, 585 LOC)
    • class primitive_iterator_t (private, 134 LOC)
    • struct internal_iterator (private, 22 LOC)
    • class iteration_proxy (private, 96 LOC)
    • class iter_impl (616 LOC)
    • class json_reverse_iterator (112 LOC)
    • class lexer (private, 848 LOC)
      • struct strtonum
    • class parser (private, 305 LOC)
    • class json_pointer (679 LOC)

I added the lines of code to support your observation that there is a lot of code that could be moved out of the way. However, I also found some ways to group things along the aspects they address. Furthermore, it would be helpful to also move everything related to CBOR/MessagePack as well as JSON Pointer/Patch out of the way:

• exceptions:
  • class exception
  • class parse_error
  • class invalid_iterator
  • class type_error
  • class out_of_range
  • class other_error
• iterators:
  • class iteration_proxy (private)
  • class primitive_iterator_t (private)
  • struct internal_iterator (private)
  • class iteration_proxy (private)
  • class iter_impl
• deserialization:
  • class lexer (private)
    • struct strtonum
  • class parser (private)
• serialization:
  • class serializer (private)
• binary formats (1789 LOC):
  • cbor (should be moved in class)
  • msgpack (should be moved in class)
• JSON pointer / patch
  • json_pointer
  • json_patch (315 LOC, should be moved in class)

I would like to discuss how to restructure this logically. Moving classes into separate files would then be the next step.

[theodelrieu]

This seems good to me!

I will have more time to discuss next week, meanwhile I'll try to work a bit on my fork.

[JoveToo]

Perhaps some of the deprecated code could also be moved do a "deprecated" namespace and require an explicit "using" to bring it back.
Document it well and it will break peoples builds only once after upgrading to a new version (which is when they should expect issues like this).

[nlohmann]

FYI: With the latest commit, I merged a feature branch with a manual lexer. That is, the code does not use a state machine generated by re2c, but a hand-written state machine. In addition, I implemented "input adapters" separate the actual input (e.g., stream, string, byte vector) from the lexer. This simplified a the code in a lot of places and avoids error-prone refilling of an intermediate buffer that was required by re2c.

[theodelrieu]

Nice to hear! Is it a good time to start splitting the basic_json class?
I'm not talking about splitting the code into multiple files.

[nlohmann]

Opened #623 as place to discuss the structure of the parsing functions.

[theodelrieu]

Hi, I've been thinking a bit about all the template arguments that basic_json takes.

The intent was to be "generic", and let users decide which string, object type (and others) they wanted to use.

One of the issue is that in the implementation, there's often lots of constraints about the API of those types.

Right now, you cannot use something else than std::string (I've even seen a function that returns std::to_string(...)).
ObjectType must have a key_type and a mapped_type, etc...

Although there are some people that may want to handle floats themselves (e.g. #596) I doubt someone ever specialized NumberBooleanType.

Furthermore, IIRC the AllocatorType just does not work.

So, my question is: Which template arguments are really relevant/useful?

[stale]

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.

[meox]

any news on this release?

[nlohmann]

I wish... I would like to get this release out this year, but I don't have much time for big changes at the moment. So far, I would rather have something released soon just to have develop and master be in sync again.

[nlohmann]

Todo:

• better document iteration_wrapper
• add all recent contributors to README
• add exception for Incorrect parse error with binary data in keys? #838
• example for throwing dump
• [ ] documentation of semver/deprecation

[nlohmann]

I shall release 3.0.0 tomorrow.