No structured bindings support?

[ghost]

Bug Report

• What is the issue you have?
  Cannot use C++17 structured bindings for iteration

• Please describe the steps to reproduce the issue. Can you provide a small but working code example?

Although this DOES work (taken from the docs):

for (json::iterator it = o.begin(); it != o.end(); ++it) {
  std::cout << it.key() << " : " << it.value() << "\n";
}

The following DOES NOT work:

for (auto&& [k,v] : o)
  std::cout << k << ":" << v << "\n";

• What is the expected behavior?
  It should do the same thing as the traditional iterator implementation

• And what is the actual behavior instead?
  Does not compile

• Which compiler and operating system are you using? Is it a supported compiler?
  clang++ 6.0 (svn tree)

• Did you use a released version of the library or the version from the develop branch?
  Yes, 3.0.1

• If you experience a compilation error: can you compile and run the unit tests?

The tests work fine, so no issues there

Feature Request

• Describe the feature in as much detail as possible.

• Include sample usage where appropriate.

[gregmarr]

You need to use the iterator_wrapper() function to use a json object in a range-based for loop. That doesn't work with the deduction guides, but you can at least get the range support. Note that this library currently only requires C++11, not C++17.

[gregmarr]

Another thing that would prevent supporting deduction guides like this is that the iteration proxy is lazy. It doesn't compute the key and value unless you ask for them. It's the same reason that it doesn't have .first and .second. It would be a significant change to support this.

[nlohmann]

Would it be a good idea to add .first / .second members to the iterator_wrapper?

[gregmarr]

There would be a performance cost to that. It probably wouldn't be huge, but it would mean that incrementing the iterator either calls std::to_string() (if it now points to an array) or copies a string (if it now points to an object) for the key, and always copies a string for the value. This would just move the operation if the user already calls key(), but add the operation if not. It would double the number of copies if the user already calls value() or add a single copy if not. If for some reason, neither key() nor value() is called, then it would all be extra work. However, that should be a rare case. The only reason I can think of off the top of my head for that is counting the number of elements, but there are better ways to do that.

[gregmarr]

Summary:

Currently:
++ is cheap.
key() is a string generate and return by move, a return by copy, or a return of an empty string
value() is a return by copy

With .first and .second:
++ is a string generate or copy or clear into .first, plus a string copy into .second and the iterator is larger by two std::string objects. As this is a local variable, no big deal.
key() is a return by copy of .first
value() is a return by copy of .second
The user can optimize by using .first and .second directly in an operation that doesn't do a copy, or they could move from them as they're just going to iterate next.

[gregmarr]

Correction: value() does not return std::string, it returns json& or const json&, so .second needs to be more like std::reference_wrapper<std::remove_reference<IteratorType::reference>::type>. That actually makes it a bit lighter weight.

Also, another point from a comment on #874

The semantics are already a bit different than iterating over a map as it allows you to iterate over an array and get the indices as strings

This means that assuming we do add items() from #874, you can do this:

json j{1,2,3,4,5};

for(auto &&[k,v] : j.items()) {
    std::cout << k << ":" << v << "\n";
}

and get output like this:

0: 1
1: 2
2: 3
3: 4
4: 5

I'm not sure whether that's a good thing or not.

[nlohmann]

I have not looked into my std::map implementation, but how is the pair generated there that is returned by the iterators?

[gregmarr]

It's what the map stores internally in its nodes.

[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.

[nlohmann]

FYI: Structured binding support has recently been merged (#1391) and will be part of the next release.