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 :)

Reply via email to