Skip to content

Cannot assign string to a json value #871

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

Closed
pps83 opened this issue Jan 19, 2019 · 4 comments
Closed

Cannot assign string to a json value #871

pps83 opened this issue Jan 19, 2019 · 4 comments

Comments

@pps83
Copy link

pps83 commented Jan 19, 2019

That's kind of crazy the problem exists in the library, but the most obvious thing doesn't seem to work:

    Json::Value jmessage;
    std::string sdp;
    jmessage["something"] = sdp;

this results in a link error:

error LNK2019: unresolved external symbol "public: class Json::Value & __thiscall Json::Value::operator=(class Json::Value const &)"

I get this error with when using # define JSONCPP_VERSION_STRING "1.6.5"

If I take current git it doesn't even compile:

2>json/json.h(298): error C2065: 'SecureAllocator': undeclared identifier

In general, those implicit constructors that construct string to Json::Value aren't a good idea IMO. Library should also provide .assign methods. In both case I use amalgamated version

@BillyGoto
Copy link

BillyGoto commented Jan 19, 2019 via email

@pps83
Copy link
Author

pps83 commented Jan 19, 2019

I disagree unless there’s a functional difference between operator= and an assign member function. I guess the parenthetical and operator precedence are different. But operator= is pretty standard!

No difference, but jsoncpp could like most containers provide .assign that does the same as operator=. That way intellisense can list available methods and you can see right away what you can use to assign a value. When it didn't work, I had to go over entire Value class to see what's available and what happens there and implicit Value ctor just confused me originally, as I was looking for an operator=.

When you said “explicit” did you mean “implicit”? It can be argued whether they are good or bad, I guess. I like them.

Yes, I meant implicit (corrected). I like them too, until they cause unexpected issues.

If we have an implicit constructor from T, we should provide operators like assignment directly from T as well, so we aren’t doing more work than we need to, converting to Value and then assigning from the temporary Value. This gets expensive for relationals where you are making a Value just to throw it out.

Supposedly passing by value and swapping it out should be ok (off course assigning directly would be better), but I have no idea why VS2015 gives a link error in this piece of code. If I change that std::string sdp; to Value sdp; then it links. By the way, that code that I'm trying to build uses ancient jsoncpp version from 2012 and had no issues, but when I updated jsoncpp then I started to get all kinds of compilation/link errors. Just in case, that code is part of chrome browser and webrtc project from google, they use jsoncpp from 2012 and didn't update.

@pps83
Copy link
Author

pps83 commented Jan 19, 2019

If I take current git it doesn't even compile:

2>json/json.h(298): error C2065: 'SecureAllocator': undeclared identifier

In general, those implicit constructors that construct string to Json::Value aren't a good idea IMO. Library should also provide .assign methods. In both case I use amalgamated version

I just tried minimal example:

#include "json/json.h"

int main()
{
    Json::Value jmessage;
    std::string sdp;
    jmessage["something"] = sdp;
}

and it works with non-amalgamated version and fails to compile with amalgamated one. So, something isn't right with the amalgamation as it results in different results.
Interesting part that I don't get linking error with the minimal example, but I do get link error when building it as part of chrome/webrtc. I didn't try to investigate, perhaps r-value references are disabled or some other compilation flags trigger it, no idea.

@baylesj
Copy link
Contributor

baylesj commented Jun 26, 2019

Closing due to inactivity, and looks like it may have been fixed in other patches.

@baylesj baylesj closed this as completed Jun 26, 2019
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

4 participants
@baylesj @pps83 @BillyGoto and others