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

"Fixed" Visual Studio constinit errors #10232

Merged
merged 1 commit into from Jul 9, 2022
Merged

Conversation

Eddie-cz
Copy link
Contributor

@Eddie-cz Eddie-cz commented Jul 8, 2022

Fixes #9698 and #10159 for now. Replaces incorrect #10231 pull request.

@google-cla
Copy link

google-cla bot commented Jul 8, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@Eddie-cz
Copy link
Contributor Author

Eddie-cz commented Jul 9, 2022

I would really like this issue was closed finally, as it is present for years and rises again and again. Why somebody without clue, why there is msc_ver ifdef, removes it, and commits ab4585a#diff-c6c960709f2bebcb680ec27d5b8c61777bcfa7081e00229e22b0fe9fc3226ee5.

@fowles
Copy link
Member

fowles commented Jul 9, 2022

We need you to agree to the cla and reply with "I signed it" to trigger the recheck before we can accept this

@fowles
Copy link
Member

fowles commented Jul 9, 2022

as for the re-rising, I am sorry about that. It got erased by one of our scripts that tries to synchronize the internal and external versions of protobuf. We have a major project ongoing to fix these and reduce the delta between them, so hopefully we will be able to finally put it to bed for good

@Eddie-cz
Copy link
Contributor Author

Eddie-cz commented Jul 9, 2022

I pushed it under my company email, instead of private one. How can I proceed with CLA? I cannot sign other CLA;-(

@fowles
Copy link
Member

fowles commented Jul 9, 2022

I kicked the script and it recognizes a CLA for you now, so we are good on that front. I will merge after the pending tests pass (usually takes ~2-3h)

@fowles fowles removed the request for review from jorgbrown July 9, 2022 17:24
@Eddie-cz
Copy link
Contributor Author

Eddie-cz commented Jul 9, 2022

I kicked the script and it recognizes a CLA for you now, so we are good on that front. I will merge after the pending tests pass (usually takes ~2-3h)

Great!

@fowles fowles merged commit 855ef0c into protocolbuffers:main Jul 9, 2022
@dudantas
Copy link

I still have this problem in visual studio (17.0) with the latest vcpkg and protobuf 3.21.

@Eddie-cz
Copy link
Contributor Author

I still have this problem in visual studio (17.0) with the latest vcpkg and protobuf 3.21.

Either apply patch manually to your vcpkg version, or make protobuf port patch request to vcpkg.

@fowles
Copy link
Member

fowles commented Jul 19, 2022

We have an upcoming 21.x release which will include this

@dudantas
Copy link

I still have this problem in visual studio (17.0) with the latest vcpkg and protobuf 3.21.

Either apply patch manually to your vcpkg version, or make protobuf port patch request to vcpkg.

ok, thanks

@davidben
Copy link
Contributor

Note this doesn't actually fix the constinit issue. It merely suppresses a compile error that was flagging a real bug in protobuf. Protobuf's constant initialization design was predicated on a bunch of patterns being constinit. MSVC and clang-cl are correct to flag it as not constinit because, on Windows, references to dllimport variables emit a static initializer. They are not actually constinit. See details in #10159.

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.

error C2127: illegal initialization of 'constinit' entity with a non-constant expression
5 participants