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

Add support for std::string_view to Json::Value::operator[] #1352

Open
sir-exasol opened this issue Nov 5, 2021 · 3 comments
Open

Add support for std::string_view to Json::Value::operator[] #1352

sir-exasol opened this issue Nov 5, 2021 · 3 comments

Comments

@sir-exasol
Copy link

Is your feature request related to a problem? Please describe.
We use std::string_view for names of object entries. However, when accessing values with Json::Value::operator[], we always have to convert the name a std::string first. This incurs an unnecessary allocation and adds boilerplate code.

Describe the solution you'd like
I would like Json::Value::operator[] to support std::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 to const char* is in general not possible, because the contents of std::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.

@BillyDonahue
Copy link
Contributor

The PR was incorrect code.
I don't think it would compile. A key variable was misspelled.

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.

@aberaud
Copy link

aberaud commented Mar 24, 2022

GCC is now moving to C++17 by default, and C++20/22 include additional support for std::string_view.

This feature would be great to improve performance and code readability!

Also simply to be compatible and interoperable with the C++ standard library...

@aberaud
Copy link

aberaud commented Mar 28, 2022

first step here: #1397

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants