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

Fix warnings when compiling with clang and -Wimplicit-fallthrough #350

Merged
merged 1 commit into from
Jun 3, 2015
Merged

Fix warnings when compiling with clang and -Wimplicit-fallthrough #350

merged 1 commit into from
Jun 3, 2015

Conversation

c0nk
Copy link

@c0nk c0nk commented May 31, 2015

No description provided.

@c0nk
Copy link
Author

c0nk commented May 31, 2015

Please apply this to the v1.0 branch too.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 013b71b on c0nk:fix-clang-implicit-fallthrough into c8c8ad4 on miloyip:master.

miloyip added a commit that referenced this pull request Jun 3, 2015
Fix warnings when compiling with clang and -Wimplicit-fallthrough
@miloyip miloyip merged commit 1f78bc1 into Tencent:master Jun 3, 2015
@pah
Copy link
Contributor

pah commented Jun 3, 2015

I don't understand this change and how this fixes anything (in correctly setup environments).

The purpose of the RAPIDJSON_PARSE_ERROR_EARLY_RETURN is to allow "skipping" of the return paths in case exceptions are used for error reporting. This change will require the explicit redefinition of RAPIDJSON_PARSE_ERROR_EARLY_RETURN_VOID in these cases as well, which is not documented now.

Can you please provide the exact places where the warning is triggered? I would prefer to address these cases explicitly.

Thanks, Philipp

@miloyip
Copy link
Collaborator

miloyip commented Jun 3, 2015

The places are actually in iterative parser. I revert the merge and fix those warnings.

@c0nk
Copy link
Author

c0nk commented Jun 3, 2015

Are users allowed to redefine RAPIDJSON_PARSE_ERROR_EARLY_RETURN and RAPIDJSON_PARSE_ERROR_EARLY_RETURN_VOID? My change simply replaced:

if (HasParseError()) { return RAPIDJSON_NOTHING; }

with:

return RAPIDJSON_NOTHING;

The parse error is set on the line before so that if is always true. The if expr was tricking clang into thinking fallthroughs were possible.

@c0nk c0nk deleted the fix-clang-implicit-fallthrough branch June 3, 2015 18:10
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

Successfully merging this pull request may close these issues.

4 participants