[lmi] static constexpr [Was: master c01b9b0 02/22: Improve concinnity]

[Greg Chicares]

On 6/16/21 1:20 AM, Greg Chicares wrote:
[...]
> Tomorrow I'll reconsider the other occurrences of "static constexpr" to
> see whether I can find any argument for retaining it anywhere.

Let's examine each case in
  git grep 'static constexpr'
in some detail. (Soon I'll commit a changeset and ask whether you
agree that it's complete.)

  monnaie.hpp:    static constexpr int cents_digits = 2;
  monnaie.hpp:    static constexpr int cents_per_dollar = 100; // std::pow(10, 
cents_digits)
  monnaie.hpp:    static constexpr amount_type max_dollars()
  currency.hpp:    static constexpr int    cents_digits     = 2;
  currency.hpp:    static constexpr double cents_per_dollar = 100.0;

These are okay as they stand. They're class "properties" that can
be queried by other code, even without a class instance.

  currency_test.cpp:    static constexpr double d0 = 
std::numeric_limits<double>::max();
  currency_test.cpp:    static constexpr double d0 = 
std::numeric_limits<double>::infinity();

These are interesting. I'm quite sure you would remove 'static' here,
for the same reason you'd remove it in the commit (c01b9b0) that
started this discussion: we don't need storage allocated to hold the
result. Here's the full context of the first instance:

  void currency_test::mete_humongous()
  {
      static constexpr double d0 = std::numeric_limits<double>::max();
      static currency const extreme = from_cents(d0);
      static currency const value   = from_cents(1234567);
      for(int i = 0; i < 100000; ++i)
          {
          currency volatile z = std::min(extreme, value);
          }
  }

What I find interesting is that making all three local variables 'static'
seemed like a good idea because it favors superficial consistency, but
that's not really best. We do want 'extreme' and 'value' to be static,
because they should be initialized once only, and thus storage must be
allocated to hold their values. However, we don't need to store 'd0',
which is used only to initialize 'extreme'.

Testing with pc-linux-gnu only, I find that the time for mete_humongous(),
with the first 'static' left intact, varies from one trial to the next;
however, removing the first 'static' eliminates that variability, causing
the reported time to be uniformly as the lowest timing without that change.

Alternative not pursued: make all those local variables global instead.
That might produce slightly better machine code, and it would remove the
likely source of the variability noted above. But it would make the code
a little harder to read, and unit tests should maximize readability.

  ihs_avsolve.cpp:        static constexpr currency C100 = 100_cents;
  ihs_basicval.cpp:    static constexpr double dbl_inf = 
std::numeric_limits<double>::infinity();
  ihs_basicval.cpp:    static constexpr double dbl_inf = 
std::numeric_limits<double>::infinity();
  zero.hpp:    static constexpr double epsilon   
{std::numeric_limits<double>::epsilon()};

Those four should clearly be changed in light of the preceding discussion.

  view_ex.cpp://  static constexpr std::string unnameable{"Hastur"};

I thought gcc-10.2.0 might allow that, but no:

/opt/lmi/src/lmi/view_ex.cpp:213:34: error: the type ‘const string’ {aka ‘const 
std::__cxx11::basic_string<char>’} of ‘constexpr’ variable ‘unnameable’ is not 
literal
  213 |     static constexpr std::string unnameable{"Hastur"};
      |                                  ^~~~~~~~~~
/usr/lib/gcc/i686-w64-mingw32/10-win32/include/c++/bits/basic_string.h:77:11: 
note: ‘std::__cxx11::basic_string<char>’ is not literal because:
   77 |     class basic_string
      |           ^~~~~~~~~~~~
/usr/lib/gcc/i686-w64-mingw32/10-win32/include/c++/bits/basic_string.h:77:11: 
note:   ‘std::__cxx11::basic_string<char>’ does not have ‘constexpr’ destructor

Maybe it'll work with a future gcc version. Until then, today's code:
    static std::string const unnameable{"Hastur"};
is plenty good enough.

  stratified_algorithms.hpp:///   static constexpr T zero {};
  stratified_algorithms.hpp:    static constexpr T zero {};
  stratified_algorithms.hpp:    static constexpr T zero {};
  stratified_algorithms.hpp:    static constexpr T zero {};
  stratified_algorithms.hpp:    static constexpr T zero {};
  stratified_algorithms.hpp:    static constexpr T zero {};

Those six cases are documented thus:

  /// Implementation note: Many function templates in this header
  /// require a zero of the appropriate type T, and accordingly define
  /// a local variable
  ///   static constexpr T zero {};
  /// in case this zero is expensive to construct. It is constructed
  /// as "T{}" rather than "T(0)" because the latter uses an explicit
  /// integer argument, which may require a converting constructor
  /// (for example, with class currency).

so 'static constexpr' seems appropriate: the constant may be
expensive to construct, so we want to construct it OAOO and
store its value.