json icon indicating copy to clipboard operation
json copied to clipboard

.value() with optional default value fails to compile

Open audaki opened this issue 2 years ago • 7 comments

Description

We upgraded from 3.10.5 to 3.11.2

Since the Upgrade we can't compile value() with optional fallback value anymore

Reproduction steps

Try to compile value with std::optional<std::string>{} as fallback. Is this still possible at all, was I using a non-supported edge-case before?

Expected vs. actual results

Compiles vs not

Minimal code example

Beforehand we said:

std::optional<std::string> customer_id;
customer_id = json.value("customer_id", std::optional<std::string>{});

This no longer compiles. Even without the assignment and I tried different versions, all fail to compile:

json.value("customer_id", std::optional<std::string>{});
json.value<std::optional<std::string>>("customer_id", std::optional<std::string>{});


### Error messages

_No response_

### Compiler and operating system

Ubuntu 22.04, Clang 14

### Library version

3.11.2

### Validation

- (haven't tested this) [ ] The bug also occurs if the latest version from the [`develop`](https://github.com/nlohmann/json/tree/develop) branch is used.
- [X] I can successfully [compile and run the unit tests](https://github.com/nlohmann/json#execute-unit-tests). **All passed**

audaki avatar Dec 02 '22 12:12 audaki

Looks like this type is failing the detail::is_getable<> test because get<> doesn't work for it. The get<> function didn't work in 3.10.5 either, so the check on value() is now stricter than it was before.

@theodelrieu This came from https://github.com/nlohmann/json/commit/74b446f5fdec1a829c9cdfdccfbd5ba873562a49 Any thoughts?

gregmarr avatar Dec 02 '22 16:12 gregmarr

According to your answer it seems like it was never intended to work.

Therefore we solved it with a lambda for now:

    auto json_to_opt_string = [](const nlohmann::json& v) -> std::optional<std::string> {
      if (v.empty())
        return {};

      return v.get<std::string>();
    };

Is there maybe another intended way in the library for this pattern that I overlooked in the documentation?

audaki avatar Dec 02 '22 18:12 audaki

PS: Maybe add a get_opt member function that is like get<T> but wraps every type in optional?

audaki avatar Dec 02 '22 18:12 audaki

I honestly don't know if it was intended or not. I was just looking at why it wasn't working now. I know we've had various requests for and PRs about better std::optional handling.

gregmarr avatar Dec 05 '22 16:12 gregmarr

Bumping this up. This issue has hit me hard when upgrading the version of this library from 2.x to 3.x in a project I'm trying to resurrect.

sgrzeszc avatar Nov 13 '23 11:11 sgrzeszc