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

Explictly use mt19937 random generator for boost 1.67. #2338

Conversation

de-vri-es
Copy link
Contributor

This should fix #2284.

Boost 1.67 switched the default boost::uuid::random_generator from boost::uuid::basic_random_generator<boost::mt19937> to boost::uuid::random_generator_pure which uses OS provided randomness.

The new one doesn't accept a PRNG as constructor argument anymore. This PR switches back to the old behaviour explicitly.

It may actually be a good idea to switch to the new behaviour, but that would require some preprocessor checks to stay compatible with boost < 1.67. This PR doesn't change anything, it just chooses the old default explicitly.

Preprocessor checks can be added instead if that has the preference.

@de-vri-es de-vri-es force-pushed the outofcore-boost167-random-generator branch from 227a161 to 2309bda Compare June 11, 2018 14:20
@de-vri-es
Copy link
Contributor Author

Hmm, the test failure seems unrelated?

@taketwo
Copy link
Member

taketwo commented Jun 11, 2018

Yeah, must be something else.

I think just using the old behavior explicitly is good enough for now. We can always add #ifdefs and switch to the new behavior if the need arises. Thanks for sending the patch!

@taketwo taketwo merged commit b588c54 into PointCloudLibrary:master Jun 11, 2018
@de-vri-es
Copy link
Contributor Author

Thanks for the quick review! :)

@de-vri-es de-vri-es deleted the outofcore-boost167-random-generator branch June 12, 2018 16:59
@SergioRAgostinho SergioRAgostinho changed the title outofcore: Explictly use mt19937 random generator for boost 1.67. Explictly use mt19937 random generator for boost 1.67. Aug 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fails to build against Boost 1.67
3 participants