[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-API] [PATCH] imported patch removing-warnings

On 10/11/10 10:51, Rok Strnisa wrote:
I know that an empty assignment with no side-effects can be removed, and I did 
it in some of the most obvious cases. However, I think it's better to be 
conservative here, rather than introducing hard-to-find bugs, which occur due 
to subtle side-effects missing.

But, you are introducing future hard-to-find bugs. if nothings change since last i look, partial applications warning are enabled as error (they are really the nastiest kind of errors).

when using let _ = .. you're explicitely telling the type system, that you don't care what the value is. as such it can't treat a unused partial application as a problem, because you're explicitely told him to ignore it. Hence in the future if someone add arguments to some function that happens to be on the right side of the let (_) = .., suddenly the value becomes of type "a -> b" instead of "a", which because of the let _ binding will be ignored (and because it's partial, the function is not applied, and possible side effects are then missing).

This is why you need to type your ignored let binding with a let (_ : type expected) if you want to fix the warning *properly*.

as a convenience there is a ignore_{int|string|...} in stdlib/pervasiveext for common type that can be ignored.

This patch is in fact only a start of removing the sea of warnings that get 
generated while compiling xen-api.hg. Currently, there are so many warnings 
that nobody pays attention to them any more, or notices that they generated a 
new one --- the whole purpose of warnings is lost. The goal is to reach zero 
warnings, and then enable the warnings-as-errors flag.

Therefore, I think the patch is safe as-is, but feel free to remove assignments 
you are definitely sure don't give side-effects _and_ were not meant for 
something else.

obviously, I want to make it clear, this is very useful work to remove the warnings.

You might also want to turn off verbose building in omake that only print when there's a warning. it gives more incentive for everyone to fix their own warnings (provided you don't have too many in the first place).


xen-api mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.