Re: [boost] Current Guidance on Compiler Warnings?

Subject: Re: [boost] Current Guidance on Compiler Warnings?
From: Richard Hodges (hodges.r_at_[hidden])
Date: 2018-11-24 05:48:04

On Fri, 23 Nov 2018 at 20:59, Emil Dotchevski via Boost <
boost_at_[hidden]> wrote:

> On Thu, Nov 22, 2018 at 12:56 AM Jayesh Badwaik via Boost <
> boost_at_[hidden]> wrote:
> >
> > On Monday, 19 November 2018 21:02:55 CET Emil Dotchevski via Boost wrote:
> > > In that context there are probably legal reason for zero-warning
> policy,
> > > but it is not true that lack of warnings means fewer errors, in fact it
> > > could easily lead to more errors. For example warnings about implicit
> > > conversions are frequently "fixed" by replacing the implicit conversion
> > > with explicit conversion (cast). But the two are semantically very
> > > different, and therefore one of them is incorrect, and very likely that
> is
> > > the explicit one.
> >
> > Didn't know that. Can I see an example where the explicit warning is
> > incorrect as opposed to implicit warning?
>
> I'm guessing you want an example where the explicit conversion is
> incorrect, and you want to use an implicit conversion even though you get a
> warning. Here:
>
> unsigned f();
>
> void g( int x )
> {
>   if( x < f() ) //warning C4018: '<': signed/unsigned mismatch
>   {
>     ....
>   }
> }
>
> So you change that to:
>
> if( static_cast<unsigned>(x) < f() )
>

Better yet, provide a little library boilerplate and make the conversion
explicit, correct and checkable at compile time:

void g( int x )
{
  using result_type = decltype(f());
  if(auto x_ = convert(to_type<result_type>, x) ; x_ < f())
  {
    foo();
  }
}

The library code would look something like this:

namespace conversion
{
  // the concept of converting a From to a To
  template<class From, class To>
  struct convert_op;
}

namespace keywords
{
  template<class T> struct to_type_t{};
}

template<class T> constexpr auto to_type = keywords::to_type_t<T>{};

// provide the convert interface
template<class To, class From>
auto convert(keywords::to_type_t<To>, From&& from)
{
  // implement in terms of a deduced op
  auto op = conversion::convert_op<std::decay_t<From>, std::decay_t<To>>();
  return op(std::forward<decltype(from)>(from));
}

// now specialise for particular cases (or patterns of cases)
namespace conversion
{
  template<>
  struct convert_op<int, unsigned int>
  {
    template<class In>
    auto operator()(In&& in) const -> unsigned int
    {
      assert(in >= 0);
      return static_cast<unsigned int>(in);
    }
  };
}

https://godbolt.org/z/LeMZP7

>
> Then under refactoring both f and g get changed:
>
> unsigned long f();
>
> void g( long x )
> {
>   if( static_cast<unsigned>(x) < f() )
>   {
>     ....
>   }
> }
>
> And now you probably have a bug, and no warning. I say probably, because if
> this was my code, I'd know the cast is correct because my choice of
> implicit vs. explicit conversion does not depend on what warnings I get
> from the compiler, I do what is right. But if you've instructed your
> programmers to use casts to silence warnings, you never know why that cast
> sits there.
>
> Assuming the implicit conversion is correct (it probably is, but you should
> always know), this should be written as:
>
> assert(x>=0);
> if( x < f() )
>
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>

-- 
Richard Hodges
hodges.r_at_[hidden]
office: [phone]
home: [phone]
mobile: [phone] (this will be *expensive* outside Andorra!)
skype: madmongo
facebook: hodges.r