octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: ‘pi’ shadows a global declaration


From: Rik
Subject: Re: ‘pi’ shadows a global declaration
Date: Mon, 10 Jul 2017 11:18:20 -0700

On 07/10/2017 11:01 AM, John W. Eaton wrote:
> On 07/10/2017 01:24 PM, Rik wrote:
>
>> We guarantee in configure.ac that these constants are available.  See
>> the following check:
>>
>> if test $octave_cv_header_math_defines = yes; then
>>   AC_DEFINE(HAVE_MATH_DEFINES, 1,
>>     [Define to 1 if defines such as M_PI are available in math.h])
>> else
>>   AC_MSG_ERROR([MATH DEFINES in math.h such as M_PI are required to
>> build Octave])
>> fi
>>
>>>
>>> So we could just use the macros, but I'm not sure that we should
>>> depend on them as they are not standard for C++ code.  Maybe we should
>>> define our own header file for math constants and use them consistently?
>>
>> Almost everywhere we care to build Octave these constants are a part of
>> <cmath>.  We could do this, but we really haven't had any problem to
>> date.  I would let sleeping dogs lie.
>>
>>>
>>> Whatever we do, I'd prefer to use the same constants in all Octave code.
>>
>> I agree.  At the moment, I would prefer to get rid of these constants in
>> favor of the generic ones in math.h/cmath.  I already started doing that
>> over on the liboctave side about two weeks ago in this cset:
>>
>> changeset:   23682:6fe13cd3543c
>> user:        Rik <address@hidden>
>> date:        Fri Jun 23 12:32:19 2017 -0700
>> files:       liboctave/numeric/lo-mappers.cc
>> liboctave/numeric/lo-mappers.h liboctave/numeric/lo-specfun.cc
>> description:
>> Clean up lo-mappers.h, lo-mappers.cc.
>>
>> Here is a representative change from that cset:
>>
>>      FloatComplex
>>      log2 (const FloatComplex& x)
>>      {
>> -#if defined (M_LN2)
>> -      static float ln2 = M_LN2;
>> -#else
>> -      static float ln2 = log (2.0f);
>> -#endif
>> -      return std::log (x) / ln2;
>> +      return std::log (x) / static_cast<float> (M_LN2);
>>      }
>
> Thanks for looking at this.  Instead of failing in configure and
> duplicating the #ifdef logic everywhere they are needed, why not just
> ensure that the necessary constants are available somewhere?

My changeset eliminated the #ifdef logic.  That was the simplification in
the  C++ files.  The configure test ensures that the necessary constants
are available, and we've never had a reported problem with the systems we
build for.  It's not quite lazy, but there doesn't seem to be a need to
overengineer this.  If someone does find a platform where this fails then
it will be early on in configure and we can decide what we want to do. 
There are options available at that time.  We could get math.h from gnulib
or define the constants ourselves.

--Rik



reply via email to

[Prev in Thread] Current Thread [Next in Thread]