Make Value( bool value ) etc explicit

[xingjy10]

when passing a Json::Value pointer to a const Json::Value &;
compiler will automatically transform the pointer to bool then to a json::value;

void LogJson(const Json::Value& value) {
  LOG(ERROR) << value.toStyledString();
}
int main() {
  Json::Value json = Json::Value(Json::objectValue)
  const Json::Value *json_p = &json;
  LogJson(json);
  LogJson(json_p);
}

the first LogJson will log a "{}"
the second LogJson will log a "true", caused by implict conversation

Expecte the second LogJson will compile fail.

[BillyDonahue]

I think the implicit conversions from various types to Json::Value are very useful to users.
It's not always as obviously incorrect as with the LogJson example.

People do have to be careful and don't pass pointers where values are expected. This comes up a lot in API designs and the guard rails always end up being incomplete and adding complexity. Jsoncpp is just this one library and can only Nerf the language so much.

If we wanted to eliminate pointers from implicit Value constructors, maybe we can provide a
Value(T*)=delete, but carefully because we actually DO want char*. Even then it feels risky.

[xingjy10]

i think a Value(T*)=delete would be useful and with minimal change.

[jovere]

I was coming here to put this as an exact issue. I had code like the following compile without any problems whatsoever, since it was implicitly converted to a bool:

class B {
    public:
    void foo() {}
    void foo(int b) {}
};

int main() {

Json::Value invalid;
// incorrect and should never happen, since B isn't even a Json::Value, but the compiler takes it happily
invalid["data"] = new B();

}

Almost all lint checkers say to make all single parameter constructors explicit for this key reason. I'm amazed that this has been an issue for over 3 years and hasn't been addressed. We should be making the tools work for us in having safer code, not working against us.