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

Bug in writing #5

Closed
Zapmobilegames opened this issue Apr 12, 2014 · 6 comments
Closed

Bug in writing #5

Zapmobilegames opened this issue Apr 12, 2014 · 6 comments
Labels

Comments

@Zapmobilegames
Copy link

Hello,
this is the warning I got using your release (never seen on the old official release 0.11):
http://stackoverflow.com/questions/23016836/rapidjson-fix-a-bug-in-writing

@pah pah added the question label Apr 12, 2014
@pah
Copy link
Owner

pah commented Apr 12, 2014

The line mentioned in your stackoverflow post (document.h:553) doesn't show a conversion to bool in the current master branch here. Can you please add the referenced line from your copy of the code to this issue? This could help to silence the warning in the future.

In general, MS Visual C++ warning C4800 is a false positive in almost all cases. It just tells you that an integer is converted to a boolean value (true or false) implicitly by the language rules of C++, which causes some extra masking during run-time. In practice, it's not a real "performance issue".

@Zapmobilegames
Copy link
Author

Thank you Philip, there are two lines with the same warning:

case kObjectType:
handler.StartObject();
for (Member* m = data_.o.members; m != data_.o.members + data_.o.size; ++m) {
handler.String(m->name.data_.s.str, m->name.data_.s.length, // <--- this line

m->name.flags_ & kCopyFlag);
m->value.Accept(handler);
}
handler.EndObject(data_.o.size);
break;

And this one:

case kStringType:
handler.String(data_.s.str, data_.s.length, flags_ & kCopyFlag); // <--- this line
break;

Do you suggest should I change the document.h with the latest one on this git?

Edit: rapidjson\include\rapidjson\document.h on this master repo is the same I'm using and the warning is right at line 553 and line 567. Did you mean about another master?

@pah pah closed this as completed in fe0d44a Apr 13, 2014
@pah
Copy link
Owner

pah commented Apr 13, 2014

Sorry, I had a local clone (on another machine) that was not up-to-date with master. I've now suppressed the warning by explicitly forcing the flags_ & kCopyFlag expressions to bool.

@Zapmobilegames
Copy link
Author

Philip,
I'm still having the same warning. I also changed this

struct String {
    const Ch* str;
    SizeType length;
    unsigned hashcode;  //!< reserved
};  // 12 bytes in 32-bit mode, 16 bytes in 64-bit mode

to this:

struct String {
    const Ch* str;
    SizeType length;
    bool hashcode;  //!< reserved
};  // 12 bytes in 32-bit mode, 16 bytes in 64-bit mode

without success.

Zapp

2014-04-13 9:17 GMT+02:00 Philipp A. Hartmann notifications@github.com:

Sorry, I had a local clone (on another machine) that was not up-to-date
with master. I've now suppressed the warning by explicitly forcing the flags_
& kCopyFlag expressions to bool.

Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-40301329
.

Website Zapmobilegames http://www.zapmobilegames.com
Like us on Facebook https://www.facebook.com/Zapmobilegames
Follow us on Youtubehttp://www.youtube.com/subscription_center?add_user=Zapmobilegames
Follow us on Twitter: @Zapmobilegames https://twitter.com/Zapmobilegames

@pah pah reopened this Apr 13, 2014
@pah
Copy link
Owner

pah commented Apr 13, 2014

Can you test, whether changing the expressions

bool(flags_ & kCopyFlag)

to

(flags_ & kCopyFlag) != 0

silences the warning?

According to the MSDN, this should fix it,
see http://msdn.microsoft.com/en-us/library/b6801kcy.aspx

@Zapmobilegames
Copy link
Author

You got it! Nice one! :)

Zapp

2014-04-13 10:45 GMT+02:00 Philipp A. Hartmann notifications@github.com:

Can you test, whether changing the expressions

bool(flags_ & kCopyFlag)

to

(flags_ & kCopyFlag) != 0

silences the warning?

Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-40302642
.

Website Zapmobilegames http://www.zapmobilegames.com
Like us on Facebook https://www.facebook.com/Zapmobilegames
Follow us on Youtubehttp://www.youtube.com/subscription_center?add_user=Zapmobilegames
Follow us on Twitter: @Zapmobilegames https://twitter.com/Zapmobilegames

@pah pah closed this as completed in 8fd45a2 Apr 13, 2014
pah added a commit to pah/rapidjson-upstream that referenced this issue Jun 25, 2014
Instead of always just shallowly referencing the potentially allocated
strings when calling the Handler::String function, request a copy in
case the string has been allocated from an Allocator before.

This is necessary to avoid double free()s of the string memory,
especially when using the Handler to create a deep copy of a Value.

The explicit comparison against '0' is done to suppress the warning
C4800 on MSVC, see pah/rapidjson#5.
pah added a commit to pah/rapidjson-upstream that referenced this issue Jun 25, 2014
Instead of always just shallowly referencing the potentially allocated
strings when calling the Handler::String function, request a copy in
case the string has been allocated from an Allocator before.

This is necessary to avoid double free()s of the string memory,
especially when using the Handler to create a deep copy of a Value.

The explicit comparison against '0' is done to suppress the warning
C4800 on MSVC, see pah/rapidjson#5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants