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 {i,o}stream support for pcg128_t #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ahh
Copy link

@ahh ahh commented Jun 26, 2017

We defined operator{<<,>>} for pcg128_t in pcg_extras, assuming Koenig
lookup would find these as needed. This isn't actually true, because
pcg128_t is a typedef, and Koenig lookup doesn't apply to the
namespaces of typedefs. We would need to define those operators in
the global namespace; that's a bad idea.

Instead, work around it. Define explicit Input and Output functions
in the namespace, which in turn default to operator<< and operator>>
if they need to, and explicitly use them in engine printing/parsing
functions.

We defined operator{<<,>>} for pcg128_t in pcg_extras, assuming Koenig
lookup would find these as needed.  This isn't actually true, because
pcg128_t is a typedef, and Koenig lookup doesn't apply to the
namespaces of typedefs.  We would need to define those operators in
the global namespace; that's a bad idea.

Instead, work around it.  Define explicit Input and Output functions
in the namespace, which in turn default to operator<< and operator>>
if they need to, and explicitly use them in engine printing/parsing
functions.
@ahh ahh mentioned this pull request Jun 26, 2017
@ahh
Copy link
Author

ahh commented Jun 26, 2017

Note that I do not intend for this to merged as-is; it's incomplete (I need to do the same thing for uint8 handling and for extended-generator printing. Figured I'd demonstrate what the approach looks like and see if we can come to a consensus before I wrote out all the details.

Thoughts?

pcg_extras::Output(out, rng.increment());
out << space;
pcg_extras::Output(out, rng.state_);
out << space;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this change is intentional: the current parsing routine for pcg128_t is actually broken at EOF. (This only showed up with this change, as it's not used anywhere currently.

@dhardy
Copy link

dhardy commented Jul 2, 2019

I would love for this to be merged (I would like checkpointing with pcg64). @imneme would you be happy to merge this code, if @ahh completes it?

It is of course unfortunate not to be able to use operator<< and operator>> but since this only affects a small amount of code, the proposed solution is perfectly adequate in my opinion.

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