[libc++] add floating point type check for uniform real distribution

[changkhothuychung]

Created a PR to fix this issue - #62433

[changkhothuychung]

Can anyone tell me what is the issue with the Modular Build? I dont quite understand. How should I fix this?

[tschuett]

The error message says that there is a missing include for std::is_floating_point.

[changkhothuychung]

@philnik777 phik
can you let me know how I can fix the test file? I dont understand the error log and have no idea how to fix.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[AdvenamTacet]

@NhatNguyen1810 could you add a better description to the test? I looked at that file first and wasn't sure what's the purpose without reading the whole test carefully.

[changkhothuychung]

@NhatNguyen1810 could you add a better description to the test? I looked at that file first and wasn't sure what's the purpose without reading the whole test carefully.

I added more descriptions to the test file.

[changkhothuychung]

@philnik777 can you review when you have time and let me know if it looks good to be approved? There is a workflow awaiting approval, can you run it for me?

Thanks!

[philnik777]

Thanks for working on this! Some minor comments.

[changkhothuychung]

@philnik777 phik
can you take a quick look with my change? Basically I follow this file to do the regex matcher. I can't find a similar case where it does not use the regex. Let me know what I should put without the regex matcher. Thanks.

https://github.com/llvm/llvm-project/blob/main/libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add.verify.cpp#L49

[ldionne]

We have a utility called __libcpp_random_is_valid_urng in libcxx/include/__random/is_valid.h -- should we instead create something similar to it and handle http://eel.is/c++draft/rand.req.genl more generally?

[AdvenamTacet]

@NhatNguyen1810 you can see error report by clicking details next to the buildkite and then on the job which failed. It looks like your code works with GCC only. You can find details here: https://buildkite.com/llvm-project/libcxx-ci/builds/31338#018b8dda-3da3-4049-af36-d3bbe1bf054c

FAIL: llvm-libc++-shared.cfg.in :: libcxx/random/random.uniform.real/traits_mismatch.verify.cpp (1664 of 9610) - that's the test which failed.

# .---command stderr------------
--
  | # \| error: 'expected-error' diagnostics seen but not expected:
  | # \|   File /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-6be6c9a4344c-1/llvm-project/libcxx-ci/build/generic-cxx26/include/c++/v1/__random/uniform_real_distribution.h Line 31: static assertion failed due to requirement 'is_floating_point<int>::value': result_type must be floating type
  | # \|   File /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-6be6c9a4344c-1/llvm-project/libcxx-ci/libcxx/test/libcxx/random/random.uniform.real/traits_mismatch.verify.cpp Line 20: cannot find end ('}}') of expected string
  | # \| 2 errors generated.
  | # `-----------------------------

[changkhothuychung]

@philnik777 phil
can you take a look when you have time? thanks.

[changkhothuychung]

@mordante mordanta
there are timed out and failed tests that are not related to my PR at all. Can you take a look and let me know how to fix this? Thanks.

[AdvenamTacet]

It's a known problem at the moment. There is no need to take any action at the moment.

[ldionne]

LGTM with latest changes. I will merge once CI is done.

[changkhothuychung]

@ldionne ld
there was an error with code formatter, I made a push to fix it. Does everything look good to be merged now? Can you help me merge? Thanks.

[EricWF]

Please give me 24 hours to test this change at Google to see what the impact is.

[ldionne]

@changkhothuychung You have a setting on your Github account to "hide your email". Please disable it, otherwise upon committing this the email address will be [email protected]. We're trying to prevent that from happening for commit attribution purposes.

[changkhothuychung]

@changkhothuychung You have a setting on your Github account to "hide your email". Please disable it, otherwise upon committing this the email address will be [email protected]. We're trying to prevent that from happening for commit attribution purposes.

Do you see my email now? it should show up as [email protected], thanks!

[ldionne]

Yes I can see it now, thanks.

@EricWF Do you have any results internally? We've been sitting on this for a while and I'd like to merge it by EOD if possible, since it looks otherwise good to go.

[EricWF]

@ldionne I have to run the tests off hours, they're set up to run tonight.

[ldionne]

Sounds good, I'll check again tomorrow morning or mid-day. I'm eager to close this PR :)

[EricWF]

@ldionne For one reason or another, the test runs got aborted last night. I'm rerunning them again tonight.

[ldionne]

@ldionne For one reason or another, the test runs got aborted last night. I'm rerunning them again tonight.

Do you have any news about this?

[EricWF]

Yes, there are a few users in Google3. I would like to clean them up before this lands, but that will take until Wednesday. None of them seem show-stopping though. All of the usages are weird, but otherwise fine, code.

If you're so inclined, I would love the extra time. Otherwise, this LGTM.

[EricWF]

I would ask kindly that you wait a day or two, but that's only a request.

LGTM.

[ldionne]

Cool, thanks a bunch for the information. I'll land this on Thursday. This is not an emergency at all but it would be good to get it done. Hopefully that should give you enough time.