Post C++20 improvements

[Ansariel]

• Make eligible inline functions in llmath.h constexpr (held back this change to avoid conflict with std::lerp change)
• Replace boost reverse foreach with range view adoptor

@nat-goodspeed C++20 now contains several mathematical constants that are currently manually defined in the viewer in llmath.h. What is your thought about replacing the manual specified values with the ones from https://en.cppreference.com/w/cpp/numeric/constants ?

[Ansariel]

@marchcat Are we using an older GCC version on Linux that doesn't like range view adopters?

[marchcat]

Looks like we're using gcc12, which should be fine. However, I haven't checked it closely. We could probably try to bump up the version, like we did for xcode.

[AiraYumi]

@Ansariel Linux GHA Build uses clang14. Therefore, even if the build is successful with gcc, it may fail with GHA Build.

[Ansariel]

@Ansariel Linux GHA Build uses clang14. Therefore, even if the build is successful with gcc, it may fail with GHA Build.

The GHA build log says something else though.

[AiraYumi]

This is a correct message because clang uses gcc libraries (glibc) and header files internally.

[RyeMutt]

Yeah this is due to Ubuntu 22.04 being GCC-12...Bumping the OS version is possible but would result in quite a few linux distros no longer working with the GHA builds.

[Ansariel]

Yeah this is due to Ubuntu 22.04 being GCC-12...Bumping the OS version is possible but would result in quite a few linux distros no longer working with the GHA builds.

Great! You just described the reason why Linux will never play a major role in desktop computing... 🙄

[nat-goodspeed]

C++20 now contains several mathematical constants that are currently manually defined in the viewer in llmath.h. What is your thought about replacing the manual specified values with the ones from https://en.cppreference.com/w/cpp/numeric/constants ?

In general, I'm in favor of replacing (e.g.) Boost or Linden homebrew containers and functions with std equivalents. This particular change gives me pause for a couple reasons. One is that of course cppreference.com, like the Standard itself, describes these names in terms of their abstract mathematical properties -- not their specific values. That makes it a little tricky to compare (e.g.) F_PI with std::numbers::pi_v<float>.

One could write a little test program that subtracts corresponding constant definitions on each of our platforms and reports the magnitude of difference. It would surprise me a lot if it's zero in all cases.

But the real issue isn't the size of the difference in the constants themselves -- it's the divergence in the results once we've turned the crank on any number of (iterations of) different formulae involving those constants. How small a divergence is too small to accumulate to a user-visible change in the behavior of ... something?

I can't help remembering horses starving due to an inadvertent change to the friction constant. Of course a viewer-side change is less pervasive than a server-side change; on the other hand people do get really worked up about small changes in (e.g.) sky brightness.

So my thought is to proceed with caution. Let's change the definitions of the Linden constants in llmath.h to reference the std names (or expressions involving the std names, as appropriate) and gauge response to an RC.

[nat-goodspeed]

n.b. Do we seriously need all three of F_APPROXIMATELY_ZERO, F_ALMOST_ZERO and FP_MAG_THRESHOLD?

[AiraYumi]

Build fails on 24.04 (gcc 13.2 or later).

In file included from /home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/attribute/closure.hpp:21,
                 from /home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/attribute.hpp:35,
                 from /home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/include/classic_attribute.hpp:11,
                 from /home/runner/work/viewer/viewer/indra/llmath/llcalcparser.h:30,
                 from /home/runner/work/viewer/viewer/indra/llmath/llcalc.cpp:31:
In member function ‘void phoenix::impl::closure_frame_holder<FrameT>::set(frame_t*) [with FrameT = phoenix::closure_frame<phoenix::closure<float, phoenix::nil_t, phoenix::nil_t, phoenix::nil_t, phoenix::nil_t, phoenix::nil_t> >]’,
    inlined from ‘phoenix::closure_frame<ClosureT>::closure_frame(const ClosureT&) [with ClosureT = phoenix::closure<float, phoenix::nil_t, phoenix::nil_t, phoenix::nil_t, phoenix::nil_t, phoenix::nil_t>]’ at /home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/phoenix/closures.hpp:226:21,
    inlined from ‘boost::spirit::classic::closure_context<ClosureT>::closure_context(const ClosureT&) [with ClosureT = LLCalcParser::value_closure]’ at /home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/attribute/closure.hpp:75:11,
    inlined from ‘boost::spirit::classic::closure_context_linker<ContextT>::closure_context_linker(const ParserT&) [with ParserT = boost::spirit::classic::impl::rule_base<boost::spirit::classic::rule<boost::spirit::classic::scanner<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<> > >, boost::spirit::classic::closure_context<LLCalcParser::value_closure>, boost::spirit::classic::nil_t>, const boost::spirit::classic::rule<boost::spirit::classic::scanner<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<> > >, boost::spirit::classic::closure_context<LLCalcParser::value_closure>, boost::spirit::classic::nil_t>&, boost::spirit::classic::scanner<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<> > >, boost::spirit::classic::closure_context<LLCalcParser::value_closure>, boost::spirit::classic::nil_t>; ContextT = boost::spirit::classic::closure_context<LLCalcParser::value_closure>]’ at /home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/attribute/closure_context.hpp:37:17,
    inlined from ‘typename boost::spirit::classic::parser_result<DerivedT, ScannerT>::type boost::spirit::classic::impl::rule_base<DerivedT, EmbedT, T0, T1, T2>::parse(const ScannerT&) const [with ScannerT = boost::spirit::classic::scanner<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<> > >; DerivedT = boost::spirit::classic::rule<boost::spirit::classic::scanner<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<> > >, boost::spirit::classic::closure_context<LLCalcParser::value_closure>, boost::spirit::classic::nil_t>; EmbedT = const boost::spirit::classic::rule<boost::spirit::classic::scanner<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<> > >, boost::spirit::classic::closure_context<LLCalcParser::value_closure>, boost::spirit::classic::nil_t>&; T0 = boost::spirit::classic::scanner<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<> > >; T1 = boost::spirit::classic::closure_context<LLCalcParser::value_closure>; T2 = boost::spirit::classic::nil_t]’ at /home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/core/non_terminal/impl/rule.ipp:173:17:
/home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/phoenix/closures.hpp:175:38: error: storing the address of local variable ‘context_wrap’ in ‘*(phoenix::impl::closure_frame_holder<phoenix::closure_frame<phoenix::closure<float, phoenix::nil_t, phoenix::nil_t, phoenix::nil_t, phoenix::nil_t, phoenix::nil_t> > >*)this.phoenix::impl::closure_frame_holder<phoenix::closure_frame<phoenix::closure<float, phoenix::nil_t, phoenix::nil_t, phoenix::nil_t, phoenix::nil_t, phoenix::nil_t> > >::frame’ [-Werror=dangling-pointer=]
  175 |         void set(frame_t *f) { frame = f; }
      |                                ~~~~~~^~~
In file included from /home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/attribute/closure.hpp:16:
/home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/core/non_terminal/impl/rule.ipp: In member function ‘typename boost::spirit::classic::parser_result<DerivedT, ScannerT>::type boost::spirit::classic::impl::rule_base<DerivedT, EmbedT, T0, T1, T2>::parse(const ScannerT&) const [with ScannerT = boost::spirit::classic::scanner<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<> > >; DerivedT = boost::spirit::classic::rule<boost::spirit::classic::scanner<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<> > >, boost::spirit::classic::closure_context<LLCalcParser::value_closure>, boost::spirit::classic::nil_t>; EmbedT = const boost::spirit::classic::rule<boost::spirit::classic::scanner<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<> > >, boost::spirit::classic::closure_context<LLCalcParser::value_closure>, boost::spirit::classic::nil_t>&; T0 = boost::spirit::classic::scanner<__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >, boost::spirit::classic::scanner_policies<boost::spirit::classic::skipper_iteration_policy<> > >; T1 = boost::spirit::classic::closure_context<LLCalcParser::value_closure>; T2 = boost::spirit::classic::nil_t]’:
/home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/core/non_terminal/impl/rule.ipp:173:17: note: ‘context_wrap’ declared here
  173 |                 BOOST_SPIRIT_CONTEXT_PARSE(
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/core/non_terminal/rule.hpp:33,
                 from /home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/core.hpp:41,
                 from /home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/include/classic_core.hpp:11,
                 from /home/runner/work/viewer/viewer/indra/llmath/llcalcparser.h:31:
/home/runner/work/viewer/viewer/build-linux-x86_64/packages/include/boost/spirit/home/classic/core/non_terminal/impl/rule.ipp:169:41: note: ‘this’ declared here
  169 |             parse(ScannerT const& scan) const
      |                                         ^~~~~

[Ansariel]

New version: I excluded the changes in lllayoutstack.cpp for now and instead included the idea @nat-goodspeed to change lltrunc to use narrow instead, which now is also constexpr and therefore allows lltrunc to become constexpr as well.