feat: support std::string_view in Value API

[evalon32]

This adds direct support for std::string_view when available (C++17 and above). The current API can be used with std::string_view via the low-level two-pointer methods, but is not ergonomic. E.g., compare:

Json::Value node;
std::string foo, bar, baz;
std::string_view foo_sv, bar_sv, baz_sv;

// Efficient & readable:
node[foo][bar][baz];

// Less efficient, less readable:
node[std::string(foo_sv)][std::string(bar_sv)][std::string(baz_sv)];

// Efficient, but a lot less readable:
*node.demand(foo_sv.data(), foo_sv.data() + foo_sv.size())
    ->demand(bar_sv.data(), bar_sv.data() + bar_sv.size())
    ->demand(baz_sv.data(), baz_sv.data() + baz_sv.size());

// After this change, efficient & readable:
node[foo_sv][bar_sv][baz_sv];

Change details (only with C++17 or above)

• The constructor can take a std::string_view parameter. The existing
  overloads taking const std::string& and const char* are still necessary
  to support assignment from those types.
• operator[], get(), isMember() and removeMember() take a
  std::string_view parameter. This supersedes the overloads taking
  const std::string& and const char*. The overloads taking a pair of
  pointers (begin, end) are preserved for source compatibility.
• getString() has an overload with a std::string_view output parameter.
  The one with a pair of pointers is preserved for source compatibility.

[yurykats]

/assign @baylesj

[baylesj]

I think this works since it's behind a define and conditionally enabled. Ideally we would be able to have the string_view isMember contain the actual logic and use modern C++ in the find implementation.

[bcsgh]

FWIW: I suspect this resolved #1352 ??

[beevvy]

Hi @evalon32, @baylesj,

I just noticed this commit in the master branch, and I think there's a possible problem. jsoncpp is built with -std=c++11 by default, which means that when building json_value.cpp, JSONCPP_HAS_STRING_VIEW is never defined. That means that the newly added member functions won't be part of the resulting library. (I haven't verified it.)

The member functions that are compiled into the library cannot be conditionally enabled. Conditional code should be header-only, for example as inline member functions implemented in terms of other, always enabled member functions.

[evalon32]

That's a good point -- if the library and its user are compiled with different C++ versions, we'll get linker errors. I'll try to put together a follow-up PR to fix that.

[bcsgh]

Outside of plugins, are there actually any cases that really need link compatibility for libraries where the consumer has access to the source? Sure there are legacy reason to continue to support it, but are there any good technical reason for the word at large to not deprecate the cases where people are depending on it?

(There are several large classes of issues that just don't exist in a build-everything-from-source-and-static-link world.)

[beevvy]

Most Linux distributions ship with a single libjsoncpp.so.26 that's used by ~all packages depending on it. For it to be usable as shared library, the ABI must be predictable. Of course, for Linux distributions' case, you could just hard depend on C++17, but that's another topic...

[bcsgh]

@beevvy the only implication of your observation is a need to deal with that reality (and that is a very valid point).

My gripe is that anyone choose to use that .so (or basically any .so expect were the point is to intentionally get different behavior from different .so's) rather than build from source and statically link. My claim is that the cases where anything else is necessarily (or even desirable) are somewhere between few and vanishing.

And FWIW, every package I've worked on that depends on JsonCpp does not depend on libjsoncpp.so and, if I have my way, none ever will.