Re: ‘pi’ shadows a global declaration

[Rik]

On 07/10/2017 09:00 AM, address@hidden wrote:

Subject:

Re: ‘pi’ shadows a global declaration

From:

"John W. Eaton" <address@hidden>

Date:

07/10/2017 06:19 AM

To:

"Dmitri A. Sergatskov" <address@hidden>, octave-maintainers <address@hidden>

List-Post:

<mailto:address@hidden>

Content-Transfer-Encoding:

quoted-printable

Precedence:

list

MIME-Version:

1.0

References:

<address@hidden>

In-Reply-To:

<address@hidden>

Message-ID:

<address@hidden>

Content-Type:

text/plain; charset=utf-8; format=flowed

Message:

2

On 07/09/2017 11:30 PM, Dmitri A. Sergatskov wrote:

Probably no a big problem. Mostly fyi:

../src/liboctave/numeric/lo-specfun.cc:3460:19: warning: declaration of
‘pi’ shadows a global declaration [-Wshadow] const float pi =
3.14159265358979323846f; ^~
../src/liboctave/numeric/lo-specfun.cc:3198:25: note: shadowed
declaration is here static const double pi = 3.14159265358979323846; ^~

Yeah, I noticed that.  We currently use a mixture of these kinds of locally defined constants and macros like M_PI that may be defined by math.h.  Since we generally want to avoid macros from C-language headers in C++ code we try to only use C++ headers instead of C headers (<cmath> instead of <math.h>) but we also define _GNU_SOURCE, which currently causes macros like M_PI to be defined, at least on some systems.

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);
     }

--Rik