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 error C3016 on MSVC with OpenMP #1527

Merged
merged 1 commit into from
Feb 11, 2016

Conversation

UnaNancyOwen
Copy link
Member

This pull request was fixed error C3016 (issue #1499).

MSVC doesn't support an OpenMP versions earlier higher than 2.0.
The index variable in an OpenMP 2.0 for statement must be a signed integral type.
The error C3016 occurs on MSVC, because std::size_t is unsigned integral type.
Fix this issue by replacing index to the 64bit signed integral type.

@taketwo
Copy link
Member

taketwo commented Feb 11, 2016

Why specifically long long, not int32_t or int64_t?

@UnaNancyOwen
Copy link
Member Author

I think that int32_t and int64_t has been implemented from C++11.
Are you sure PCL want to use the C++11?

@taketwo
Copy link
Member

taketwo commented Feb 11, 2016

On pre-c++11 compilers these types are available through Boost. We import them in pcl_macros.h and use here and there in the rest of PCL, so should be no problem.

@UnaNancyOwen
Copy link
Member Author

OK, I changed for-loop index to int64_t.

MSVC doesn't support an OpenMP versions earlier higher than 2.0.
The index variable in an OpenMP 2.0 for statement must be a signed integral type.
@taketwo
Copy link
Member

taketwo commented Feb 11, 2016

Thanks!

@taketwo taketwo closed this Feb 11, 2016
@UnaNancyOwen
Copy link
Member Author

@taketwo Why closed this pull-request? Don't merge this commit?

@taketwo taketwo reopened this Feb 11, 2016
taketwo added a commit that referenced this pull request Feb 11, 2016
Fix error C3016 on MSVC with OpenMP
@taketwo taketwo merged commit e820461 into PointCloudLibrary:master Feb 11, 2016
@taketwo
Copy link
Member

taketwo commented Feb 11, 2016

Ooooops, sorry. Thanks for the heads-up ;)

@UnaNancyOwen
Copy link
Member Author

Thanks.

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.

2 participants