Re: [lmi] Performance of bourn_cast

[Vadim Zeitlin]

On Sat, 18 Mar 2017 23:55:06 +0000 Greg Chicares <address@hidden> wrote:

GC> Anyway, I reached for a thesaurus, came across the word "bourn", and
GC> thought, of course, of
GC> 
GC>   ... Who would fardels bear,
GC>   To grunt and sweat under a weary life,
GC>   But that the dread of something after death,
GC>   The undiscovered country from whose bourn
GC>   No traveler returns, puzzles the will
GC>   And makes us rather bear those ills we have
GC>   Than fly to others that we know not of?
GC> 
GC> That tickled my fancy: no value "returns" across this fence, because an
GC> exception is thrown.

 I'm not an English literature scholar, but I'm not totally certain that
the original author has thought to imply this meaning by using this word
here.

GC> And I personally prefer uncommon but pungent words
GC> because they're easy to grep for: e.g.,
GC>   grep fardel make
GC> is more fruitfully selective than searching for a word like "install".

 Yes, I agree. But, to compensate, if you don't already know what to search
for, you have practically no chance of discovering it. So the balance is
between making the task easier for an expert (because searching for
"install" would still allow the expert to find whatever is being searched,
it would just take more time to filter out false positives) and making it
possible at all for the rest. Of course, as long as only expert(s?) work on
lmi code base, making the former choice is easy to justify...


GC> >  BTW, have you tried making the function constexpr? Regardless of the
GC> > runtime performance, this would seem to be the right thing to do 
(although,
GC> 
GC> I'm curious--why is it the right thing?

 Because it could be then used in other constexpr expressions. I don't
really see any need to use it there because C++11 uniform initialization
does the same thing, but I don't see any need to forbid it and I think the
general principle is that anything that can be made constexpr, should be.

GC> I probably don't understand this
GC> well enough yet, but I'm thinking that could be helpful if, e.g., we have
GC> an actual constant of one integer type and want to cast it to another type;
GC> but I can't think of a practical use case. OTOH, if we have
GC>   std::vector<whatever> vec(whatever);
GC>   for(int j = 0; j < bourn_cast<int>(vec.size()); ++j)
GC> then I hope the compiler would already hoist that '.size()' out of the loop;
GC> yet maybe it would help to have a constexpr there--is that what you have in
GC> mind?

 No, I don't think constexpr can help here as the expression is clearly
unknown until run-time.

GC> Or are you just saying that a function generally should be constexpr
GC> if it can be constexpr?

 Yes.

GC> I was also thinking of a case like
GC>   for(int j = 0; bourn_cast<size_t>(j) < bourn_cast<int>(vec.size()); ++j)
GC> where we'd be asking the compiler to...create and store a constant for
GC> every value in [0, vec.size()] ?

 I don't think any contemporary compiler could do anything at compile-time
here.

GC> > admittedly, it risks being a little painful in C++11...).
GC> 
GC> Apparently g++-4.9.1 wants the body to be a single return statement.

 Yes, this is the requirement in C++11.

GC> If I just insert 'constexpr', it says:
GC> 
GC> error: body of constexpr function 'constexpr To bourn_cast(From) \
GC>   [with To = long unsigned int; From = long int]' \
GC> not a return-statement

 If you have a C++17 compiler (g++-6 is available in Debian testing, and
clang-4.0 is in unstable (only, I think)), you should be able to compile
the code unchanged, except for the addition of constexpr, with it, but we
can't use it in production yet, of course.

GC> That can be worked around as follows, but is it worth it?
GC> 
GC> 
---------8<--------8<--------8<--------8<--------8<--------8<--------8<-------
GC> diff --git a/bourn_cast.hpp b/bourn_cast.hpp
GC> index ab89b1e..451f8a8 100644
GC> --- a/bourn_cast.hpp
GC> +++ b/bourn_cast.hpp
GC> @@ -70,7 +70,7 @@
GC>  /// which may be an independent redesign.
GC>  
GC>  template<typename To, typename From>
GC> -inline To bourn_cast(From from)
GC> +inline constexpr To bourn_cast(From from)
GC>  {
GC>  #   if defined __GNUC__
GC>  #       pragma GCC diagnostic push
GC> @@ -81,13 +81,14 @@ inline To bourn_cast(From from)
GC>      static_assert(  to_traits::is_specialized, "");
GC>      static_assert(from_traits::is_specialized, "");
GC>  
GC> -    if(! to_traits::is_signed && from < 0)
GC> -        throw std::runtime_error("Cast would convert negative to 
unsigned.");
GC> -    if(from_traits::is_signed && from < to_traits::lowest())
GC> -        throw std::runtime_error("Cast would transgress lower limit.");
GC> -    if(to_traits::max() < from)
GC> -        throw std::runtime_error("Cast would transgress upper limit.");
GC> -    return static_cast<To>(from);
GC> +    return
GC> +       (! to_traits::is_signed && from < 0)
GC> +        ? throw std::runtime_error("Cast would convert negative to 
unsigned.")
GC> +      :(from_traits::is_signed && from < to_traits::lowest())
GC> +        ? throw std::runtime_error("Cast would transgress lower limit.")
GC> +      :(to_traits::max() < from)
GC> +        ? throw std::runtime_error("Cast would transgress upper limit.")
GC> +      :static_cast<To>(from);
GC>  #   if defined __GNUC__
GC>  #       pragma GCC diagnostic pop
GC>  #   endif // defined __GNUC__
GC> 
--------->8-------->8-------->8-------->8-------->8-------->8-------->8-------

 I wonder if this has any bearing on performance, in either -O0 or -O2
builds? If not, I agree that it it's not really worth it, although I still
think we might want to return to this function and stick "constexpr" in
front of it when we start using C++17.

 Regards,
VZ