On Thu, 9 Jul 2026 at 08:48, Kito Cheng <[email protected]> wrote: > > Hi Jonathan: > > Thanks for the review, inlined reply :) > > >> return std::numeric_limits<_Tp>::quiet_NaN(); > >> - else if (std::abs(__k) > _Tp(1)) > >> + else if ((std::abs(__k) > _Tp(1)) || !std::isfinite(__phi)) > > > > std::isfinite was added in C++11 so isn't available when using > > -std=c++98 -D__STDCPP_WANT_MATH_SPEC_FUNCS__ > > > > Since isnan was already used above, we only need std::isinf. Please > > use __builtin_isinf(__phi) instead. > > Oh, I don't realized std::isfinite was introduced at C++11, thanks for > the suggesion! > > > Also, please CC the libstdc++ list (as required by the > > https://gcc.gnu.org/lists.html docs) instead of individually CCing > > maintainers. > > thanks for the reminder, that's my first time to send patch to > libstdc++, I was contribute to gcc only before so I didn't notice the > rule. > > >> +// Copyright (C) 2026 Free Software Foundation, Inc. > > > > Please read > > https://gcc.gnu.org/onlinedocs/libstdc++/manual/test.html#test.new_tests > > and remove the copyright and licence headers from the new tests. > > I saw other's test has copyright and licence headers so I thought it's > necessary, yeah, I don't read that before I create testcase...:P > > >> +// 8.1.11 ellint_1 - non-finite phi must not produce UB (PR > >> libstdc++/XXXXX) > > > > Was this patch produced by AI? > > Why is there a bugzilla reference to a a non-existent bug? > > Hmm, long story and two part here: > 1. I originally planned to file a bug, but after digging deeper, I > found that it only required adding one check, so I write the patch > instead of file a bug on bugzilla, so yeah this was suppose to replace > to a real bugzilla entry. > 2. I admitted that I used AI assistance to convert this test case from > an internal report format to a libstdc++ test case format and to > proofread the commit message, but the modification is done by me after > I test with libstdc++ and boost code rather than AI generated. > > >> +void > >> +test01() > >> +{ > >> + // +infinity phi should throw domain_error, not loop forever. > >> + bool caught = false; > >> + try > >> + { > >> + volatile float r = std::ellint_1f(0.5F, > >> + std::numeric_limits<float>::infinity()); > > > > Why is this volatile? > > Simplest way from my mind to prevent ellint_1f got optimized out, > another possible way might > be use global var, but that might be optimized out due to no use. > Also, I saw a few other places in the libstdc++/testsuite use > volatile, so I guess this should be OK to use :)
I don't think it's possible to be optimized out. It's not a pure function, and throwing std::domain_error is an observable property of the function. The compiler must not optimize that out and if it does, we want the test to fail anyway! So the volatile qualifiers are doing more harm than good. I will remove those and push the v2 patch, thanks.
