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

Undef GOOGLE_PROTOBUF_MISSING_HASH after it is used. #341

Merged
merged 1 commit into from May 22, 2015
Merged

Undef GOOGLE_PROTOBUF_MISSING_HASH after it is used. #341

merged 1 commit into from May 22, 2015

Conversation

yukawa
Copy link
Contributor

@yukawa yukawa commented May 4, 2015

This is a follow up CL for df184fb
(Id937e25bbb35968ee76c92bd4a8ce6247408c443), which added
#undef GOOGLE_PROTOBUF_MISSING_HASH
where GOOGLE_PROTOBUF_MISSING_HASH macro is never defined.

With this CL, GOOGLE_PROTOBUF_MISSING_HASH macro will be cleaned up
after it is used.

This is a follow up CL for df184fb
(Id937e25bbb35968ee76c92bd4a8ce6247408c443), which added
  #undef GOOGLE_PROTOBUF_MISSING_HASH
where GOOGLE_PROTOBUF_MISSING_HASH macro is never defined.

With this CL, GOOGLE_PROTOBUF_MISSING_HASH macro will be cleaned up
after it is used.
@xfxyjwf
Copy link
Contributor

xfxyjwf commented May 22, 2015

The position of this #undef GOOGLE_PROTOBUF_MISSING_HASH doesn't seem to matter. Why would we need this change?

@yukawa
Copy link
Contributor Author

yukawa commented May 22, 2015

The biggest motivation is to avoid preprosessor warnings. IIUC, what the current code is doing is

  • Leave GOOGLE_PROTOBUF_MISSING_HASH defined if it is defined. This might be OK and I don't care so much.
  • Try to undef GOOGLE_PROTOBUF_MISSING_HASH if it is not defined. This is problematic and preprosessor shows warnings in practice.
    In short, here is what we are doing now.
#ifdef GOOGLE_PROTOBUF_MISSING_HASH
#else
#undef GOOGLE_PROTOBUF_MISSING_HASH
#endif  // GOOGLE_PROTOBUF_MISSING_HASH

And what this CL is trying to do is

#ifdef GOOGLE_PROTOBUF_MISSING_HASH
#undef GOOGLE_PROTOBUF_MISSING_HASH
#else
#endif  // GOOGLE_PROTOBUF_MISSING_HASH

Can you double check if the current location of #undef GOOGLE_PROTOBUF_MISSING_HASH is intended?

@xfxyjwf
Copy link
Contributor

xfxyjwf commented May 22, 2015

Thanks for the explanation. I didn't notice that the #undef is put in the #else block.

xfxyjwf added a commit that referenced this pull request May 22, 2015
…_HASH

Undef GOOGLE_PROTOBUF_MISSING_HASH after it is used.
@xfxyjwf xfxyjwf merged commit 55df121 into protocolbuffers:master May 22, 2015
@yukawa yukawa deleted the fix-undef-GOOGLE_PROTOBUF_MISSING_HASH branch May 22, 2015 23:21
bithium pushed a commit to bithium/protobuf that referenced this pull request Sep 4, 2023
…ROTOBUF_MISSING_HASH

Undef GOOGLE_PROTOBUF_MISSING_HASH after it is used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants