Re: lily: fix some type conversion warnings (issue 557190043 by address@hidden)

[hanwenn]

On 2020/02/02 14:41:51, dan_faithful.be wrote:
> On Feb 2, 2020, at 09:15, mailto:address@hidden wrote:
> > 
> >> I don't like this methodology, what's the difference over disabling
> >> -Wconversion
> > 
> > My selfish is reason is that it gets the warnings out of the way of
> > whomever is not interested in fixing casts, including myself.
> 
> About 1/3 of the remaining warnings are for conversions between
integer and
> floating point types.  Those are most likely to be solved with
static_cast<> in
> the long term, so I would have no objection if you approached _those_
that way.

https://codereview.appspot.com/563460043/

> (I encourage static_cast<> over C-style casts because static_cast is
easier to
> grep for and the compiler is not so see-no-evil about it.)

http://codereview.appspot.com/547560044

> > If the base compile is free of warnings, we can add -Werror to the
> > compile, and any new errors will cause a failure in our CI, which is
a
> > much better way of preventing regressions.
> 
> This is an idea I can back.  I recall that g++ supports something like
-Werror
> -Wwarning=conversion (don't have time to check) that would allow
treating most
> warnings as errors right now.

right, but that will still leave warnings that are non-actionable for
random
contributors in the compilation output, which is what I am arguing
against

> I do have a reservation when it comes to supporting many versions of
compiler. 
> I've been in the position of having to deal with false positive
warnings from
> older compilers that were not well covered in CI.  It was a drag.

Hmm.  Good point; because if we put -Werror inthe default compile flags,
we'll create 
problems for whomever is using the "wrong" version of the compiler.


https://codereview.appspot.com/557190043/