-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add support for std::string_view to Json::Value::operator[] #1352
Comments
The PR was incorrect code. It could create an ambiguity among the overloads of the operator[], which would break some users. the detection of C++17 was incorrect for the case where jsoncpp is built without c++17 but the client code is built with it. The client would have missing symbol errors. There are workarounds for these problems but it is not simple. |
GCC is now moving to C++17 by default, and C++20/22 include additional support for This feature would be great to improve performance and code readability! Also simply to be compatible and interoperable with the C++ standard library... |
first step here: #1397 |
Any chance of reviving this? And while this is being done adding the corresponding constructor |
Is your feature request related to a problem? Please describe.
We use
std::string_view
for names of object entries. However, when accessing values withJson::Value::operator[]
, we always have to convert the name astd::string
first. This incurs an unnecessary allocation and adds boilerplate code.Describe the solution you'd like
I would like
Json::Value::operator[]
to supportstd::string_view
.Describe alternatives you've considered
As mentioned an alternative is to convert the name to
std::string
first, but this is less readable and slower. Converting toconst char*
is in general not possible, because the contents ofstd::string_view
might not be null-terminated.Additional context
This feature requires C++17. jsoncpp seems to build fine with C++17 so this shouldn't be a problem as long as the feature is deactivated when compiling with a lower standard.
#999 already implemented this feature, but was closed. It is not clear to me why the PR was closed. If there is additional work necessary, I am happy to help.
The text was updated successfully, but these errors were encountered: