Re: [lmi] PATCH: simple clang -Wuninitialized-const-reference fix

[Vadim Zeitlin]

On Wed, 28 Apr 2021 22:58:59 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> I can't really say why there are two such functions, or why
GC> they're named the way they are

 Just FYI, I've added this to my TODO list, but below constexpr-izing task.

GC> >  We could do this and, arguably, we should do this. But then we should do
GC> > this and everywhere else where it could be done, it doesn't seem to single
          ^                                                           ^
         here                                                       right
GC> > out this particular function for a particular treatment.
GC> 
GC> Agreed.

 Sorry for missing so many words in the sentence above and thanks for
taking the effort to parse it nevertheless.

GC> >  And this would definitely be a very useful thing to do because the
GC> > compiler is required to detect undefined behaviour in constexpr code. 
GC> > In fact, I have a long standing item in my TODO list (severity:
GC> > "wishlist") to try running
GC> > https://github.com/trailofbits/constexpr-everything on lmi. 
GC> > I didn't think at all to talk about this in the near future, but as
GC> > you've brought this up yourself, please let me know if you think it
GC> > would be worth bringing this item further to the top of my list.
GC> 
GC> 'constexpr-everything' looks interesting.

 Yes. I haven't actually used it at all yet, so I'm somewhat afraid it
might be less magic than I hope it to be, but I'd like to at least try it.

GC> Does 'constexpr-everything' also have 'consteval-everything'
GC> capabilities?

 Judging solely from the absence of any results at

https://github.com/trailofbits/constexpr-everything/search?q=consteval

I'd say no. It also exists since a couple of years, so predates C++20 and
consteval.

GC> Surely some lmi functions should be consteval;
GC> I'm thinking of nonstd::power(T x, int n), at least for the
GC> cases we really care about:
GC>   double x = 2.0
GC>   double x = 10.0
GC> (where I was recently tempted to introduce lookup tables,
GC> but that would be a terrible mistake if we can consteval
GC> something like the existing code);

 I think we can and it there is no need to wait for constexpr-ifying
everything else in order to do it. Should I add a (higher priority) task
for this or would you prefer to try doing it yourself?

GC> and also this thing:  
GC> 
GC>     std::string look_up_scale_unit(int decimal_power)
GC>         {
GC>         return
GC>              ( 0 == decimal_power) ? ""
GC>             :( 3 == decimal_power) ? "thousand"
GC>             :( 6 == decimal_power) ? "million"
GC>             :( 9 == decimal_power) ? "billion"
GC>             :(12 == decimal_power) ? "trillion"
GC>             :(15 == decimal_power) ? "quadrillion"
GC>             :(18 == decimal_power) ? "quintillion"
GC>             : throw std::logic_error("Unnamed scaling unit.")
GC>             ;
GC>         }

 I'm not even sure off hand (and too tired to look it up) if you can throw
from a consteval function. But I'm pretty sure you should be able to static
assert when evaluating it in a compile-time context, i.e. it could be
rewritten with a chain of "if constexpr" if necessary.

GC> >  Yes, you're right, it is a better fix, thanks. I've pushed it to the
GC> > branch "stifle-unused-after-use" of vadz/lmi
GC> 
GC> Committed, tested, and pushed.

 Thanks!
VZ

pgpeB7ZoibrRW.pgp
Description: PGP signature