string_view support for object access #2685
Conversation
nlohmann
added
the
review needed
|
@gregmarr @davidstone Going from
|
| string_t value(const typename object_t::key_type& key, const char* default_value) const | ||
| #if defined(JSON_HAS_CPP_17) // avoid creating a string_t value from default_value | ||
| template < class KeyType, typename detail::enable_if_t < | ||
| detail::is_usable_as_key_type<basic_json_t, KeyType>::value, int> = 0> |
There was a problem hiding this comment.
Minor, since this template clause is the same for #if and #else you could move it outside and avoid duplicating it. If you prefer it this way, that's fine too.
|
Ugh, just saw the failure on Windows. Will take a look. |
|
Looks like it's failing with the string literal lookups. I think these two functions were here to handle the string literals. https://github.com/nlohmann/json/pull/2685/files#diff-b56a00981d8f3b87e3ce49a7eb27d36f4586d9c54c3fb628a88cfc000aa5fed4L3813-L3903 |
|
Here is the issue where they were added to fix this problem: #171 |
|
@nlohmann Have you tried putting back in those two functions? |
|
The CI still fails (https://github.com/nlohmann/json/runs/4618188061?check_suite_focus=true). Any ideas? |
nlohmann
added
state: help needed
| @@ -3816,6 +3816,19 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec | |||
| JSON_THROW(type_error::create(305, "cannot use operator[] with a string argument with " + std::string(type_name()), *this)); | |||
| } | |||
|
|
|||
| // see https://github.com/nlohmann/json/pull/2685#issuecomment-994015092 | |||
| template<typename T, std::size_t n> | |||
| reference operator[](T * (&key)[n]) | |||
There was a problem hiding this comment.
a compile-time string literal is, i believe, never a non-const reference to char, they're always const.
and this appears to be a pointer to non-const reference to array of T ?
If it helps you any, you can make an "Alias" typedef, like this:
template<typename T>
using Alias = T;
Then define the signature of your functions like
template<typename T, std::size_t n>
reference operator[](Alias<T[n]> const&)
{
return ....
}There was a problem hiding this comment.
I'm afraid I do not understand what you mean. In particular, what should go to the .... part.
There was a problem hiding this comment.
What would go into the "...." part is whatever the operator[] function should do when given a const-ref to char array.
I just don't see how the function as currently written could possible do anything. It's parameter is a reference to array of pointers, not a reference to array of values.
There was a problem hiding this comment.
Looks like I might have misdirected you a bit. The functions that were added in this commit were the single argument versions:
template<typename T>
reference operator[](T* key)
template<typename T>
const_reference operator[](T* key) const
and it was to support string literals converted to plain pointers, not the actual literals themselves:
const char* _VAR1 = "MyKey";
j[_VAR1] = 10;
char* _VAR1 = "MyKey";
j[_VAR1] = 10;
…g_view � Conflicts: � doc/mkdocs/docs/api/basic_json/contains.md � doc/mkdocs/docs/api/basic_json/find.md � include/nlohmann/json.hpp � single_include/nlohmann/json.hpp � test/src/unit-regression2.cpp
|
(I had to resolve the conflicts) |
|
See #1529 (comment). |
This PR adds support for
std::string_viewfor object access (operator[],at,value,erase).Todo:
contains) also need adjustment.Closes #1529.