<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Aug 21, 2009, at 5:51 PM, Rémi Denis-Courmont wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Le vendredi 21 août 2009 11:40:16 Pierre d'Herbemont, vous avez écrit :<br><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">The above solution is the most portable one, I think.<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">No. Some compilers generate warning on dead code, and some compilers<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">generate<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">warnings in both cases. There are no warning-free "portable"<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">solutions. The<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">only portable solution is to not turn warnings into errors. In fact,<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">turning<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">warnings into errors is not portable. Suck it up.<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Yes. That's why we are only turning on to error specific class of<br></blockquote><blockquote type="cite">warnings, not all warnings :-)<br></blockquote><br>You're missing the point. Occurences of warnings depend on the compiler, <br>compiler version, architecture, targetted run-time, optimization level (at <br>least). There is no portable definition of avoidable warnings. For instance, <br>if you enable errors on unused results, OS X will probably build fine, but <br>Linux _will_ fail. If you enable errors on incompatible pointer types, Windows <br>_will_ fail but probably not Linux. If you enable alignment upcasting errors, <br>ARM will fail, but x86 will do fine.<br><br>This particular case is a good example. GCC itself will generate different <br>warnings depending on its version and optimization level. If it detects that <br>this is dead code, it will complain about dead code. Then again, it might -<br>wrongly- complain about lack of return value.<br></div></blockquote><div><br></div><div>I see your point better, however, I still consider this to be anecdotical or less important than the warnings sanitization.</div><div><br></div><div>Also, as time advance, we'll get a better platform coverage and tuning with --enable-treat-warnings-as-error, this way.</div><div><br></div><div>This might seems like a dictatorial solution but I believe this will actually help VLC development.</div><div><br></div><div>I believe that if it's good to make sure that contributors and build bot build VLC with --enable-treat-warnings-as-error. We'll filter out warnings. It seems to me that this solution is the most realistic to force adoption. We let packagers use --disable-treat-warnings-as-error.</div><div><br></div><div>I can conceive that we could do the other way around, and if there is too much complaint we could --enable-treat-warning-as-error on build bot and find a way to get contributor to build with it. Yet, I fear that this could mean simply ignoring deadly warnings again.</div><div><br></div><div>So if It doesn't work on your specific arch platform combo:</div><div>- You don't feel like fixing it, fine add --disable-treat-warnings-as-error.</div><div>or</div><div>- You like fixing warnings and issue like this, then add a local exception for your configuration.</div><div><br></div><div>So far, among the other contributors, this change was well perceived. And their builds seems to be OK.</div><div><br></div><blockquote type="cite"><div><blockquote type="cite"><blockquote type="cite"><font class="Apple-style-span" color="#000000"><br></font></blockquote><blockquote type="cite">This is a stupid idea, even in debug mode. If you want to enable for<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">yourself,<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">it's your problem. On ARM Linux, I have unfixable warnings due to<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">alignment vs<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">cast problems. With threads cancellation, I also get dummy clobber<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">warnings:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">the only solution would be to use "volatile" which is just worse<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">than getting<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">a warning, IMHO. We also have warnings due to unused results from<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">glibc, in<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">some cases where we actually know what we are doing.<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">That's why we don't enable warning on error for such warnings.<br></blockquote><br>And how do you know that it's fine to enable them in ANY case?? Only those <br>warnings that are *always* avoidable and very likely actual errors are <br>acceptable. Sorry but unused parameters or unused variables simply do *not* <br>fit that definition. Indeed, hardly any of the recent "warnings fix" addressed <br>any actual *run-time* bug.</div></blockquote></div><div><br></div><div>My take on this: When a warning is reviewed by someone as being benign, it should be avoided, so that we don't get false warnings. That the story of the kid that falsely scream that there is wolf... :)</div><div><br></div><div>I am also not too keen (after all) of #warning to indicate FIXME or todo. This should probably in code, in comment and probably as a trac ticket.</div><div><br></div><div>Pierre.</div><br></body></html>