Skip to content

Explictly use mt19937 random generator for boost 1.67. - #2338

Merged
taketwo merged 1 commit into
PointCloudLibrary:masterfrom
fizyr-forks:outofcore-boost167-random-generator
Jun 11, 2018
Merged

Explictly use mt19937 random generator for boost 1.67.#2338
taketwo merged 1 commit into
PointCloudLibrary:masterfrom
fizyr-forks:outofcore-boost167-random-generator

Conversation

@de-vri-es

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

Hmm, the test failure seems unrelated?

@taketwo

taketwo commented Jun 11, 2018

Copy link
Copy Markdown
Member

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
Copy Markdown
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