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

[jonas . hahnfeld]

On 2020/01/30 08:37:44, hanwenn wrote:
> There are two problems that are tangentially related, and I think the
discussion
> comes from us wanting to solve different problems. 
> 
> My problem is that it is very distracting to change the C++ code base
if there
> are compiler warnings about existing code. So, I want a code base that
compiles
> with zero warnings as a starting point.
> 
> There is a different problem, which that there are some dubious casts
in our
> source code. I think that you, ie. Dan and David, want to address that
problem.
> 
> Here is a proposal that can make both of us happy:
> 
> 1) fix all warning provisorially, by adding the casts that the
compiler does
> today explicitly. We mark them with "TODO: investigate cast". Result:
the
> compile becomes warning-free
> 
> 2) go over all the "investigate cast" warnings, changing return
types/signatures
> where necessary. Result: our casts are now all kosher.
> 
> I'm happy to contribute the work for 1).
> 
> How does that sound?

I don't like this methodology, what's the difference over disabling
-Wconversion? (which I don't think would be a good idea)
I prefer what Dan does right now (btw thanks for working on this!): Pick
a warning, investigate the issue and fix it correctly. Otherwise I see
the risk that the TODOs will stick for longer than they should. So I
think it's a good idea to keep the "distracting" warnings in place until
they are correctly addressed.

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