This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Granularize the <random> header.
ClosedPublic

Authored by Quuxplusone on Nov 19 2021, 1:21 PM.

Details

Reviewers
ldionne
fwolff
Group Reviewers
Restricted Project
Commits
rG344cef6695e9: [libc++] Granularize the <random> header. NFCI.
Summary

The only minor functional change here is that I decided to remove #include <concepts> from <random>, because I don't think anyone has had time to start relying on that detail. But I left all the C++17-and-earlier superfluous headers intact — e.g. <random> still exports <vector>, because even some of our own tests rely on that. (I'd be happy to split out the removal of <concepts> into its own commit, immediately following the NFC commit.)

I didn't fully investigate the existing <__random/uniform_int_distribution.h>. It seems like it'd be possible now to eliminate that file's __independent_bits_engine and make it use std::independent_bits_engine instead; but that seems like a project for a separate PR.

Diff Detail

Event Timeline

Quuxplusone requested review of this revision.Nov 19 2021, 1:21 PM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptNov 19 2021, 1:21 PM

Since you move the stuff around anyways, maybe now it would be a good time to reformat it?

Since you move the stuff around anyways, maybe now it would be a good time to reformat it?

One answer is "As a rule, we shouldn't cut-and-paste-around huge amounts of code and change code in the same patch" (because that makes the chunks harder to diff and thus the movements harder to track).
My preferred answer, though, is "I see no reason to ever reformat this code": it looks mostly well formatted to me. (There are a few exceptions, like lines 88–89 of weibull_distribution.h, but I don't think clang-format would be likely to improve the situation even in that specific case.)

libcxx/utils/libcxx/test/dsl.py
210 ↗(On Diff #388609)

This was just a hack doing basically the same as D114010; I'll remove it before committing.

ldionne accepted this revision.Nov 22 2021, 9:28 AM

LGTM with comment.

As usual with these patches, I'm assuming you did not modify anything at all except what you called out. I'm fine with the <concepts> removal from <random> -- I agree it's unlikely people have started depending on that.

libcxx/include/__random/binomial_distribution.h
22–23

I haven't checked whether that's the case or not, but please only do this when the file uses min() or max(). If that's already the case, then ignore this comment.

This revision is now accepted and ready to land.Nov 22 2021, 9:28 AM
libcxx/include/module.modulemap