imneme / pcg-cpp

PCG — C++ Implementation
Apache License 2.0
745 stars 99 forks source link

Make engine constexpr under c++14 and higher #75

Open MBalszun opened 2 years ago

MBalszun commented 2 years ago

Would you be interested in a PR that makes most functions of this library constexpr? I haven't really investigated this yet, but it seems that one could just slab a CONSTEXPR_IN_ CPP14 -style macro in front of most functions. The only tricky bit would be to make sure that this macro is restrictive enough that it doesn't accidentally add constexpr for a compiler / c++ version combination that nominally supports e.g. c++14 constexpr, but actually has an incomplete implementation which then results in a compilation error even when the function isn't used in a constexpr context.

timoscs commented 1 year ago

@MBalszun a potentially useful hint, if you ever get around to working on such a PR: you can't make seed constexpr as-is under anything less than C++20, since it uses placement new internally (in C++20, you can use std::construct_at which does the trick).

I know this because I'm using my own fork under C++20 and wanted constexpr functionality, but I did that without macros and tested only on GCC12 so it's not directly useful for what you're suggesting.

MBalszun commented 11 months ago

@timoscs: Thanks for the hint. However, I got the impression, this repo is no longer maintained anyway and I don't see any option to open PRs.

imneme commented 11 months ago

To answer the original question, yes, I'd be interested. There is something to be said for having backwards compatibility to older versions of C++, but supporting constexpr use would be a valuable feature.

tbxfreeware commented 2 months ago

PCG uses placement new internally

Placement-new also has the unexpected side-effect of resetting the stream. With generators such as pcg32 and pcg64, the streams you select are discarded when you call either seed() or seed(itype).

This is the subject of Issue #94, where I wrote a fuller explanation.