[dev-servo] Disallowing warnings on all crates?
Hi, Tigercosmos sent a patch today adding #![deny(warnings)] to a bunch of crates[1]. Just wanted to give a heads-up / ask whether there's any objection to the change before going ahead and r+ it. Does it sound reasonable to everyone? It may make the servo-with-rust-nightly build fail a bit earlier, but probably that's good, actually. -- Emilio [1]: https://github.com/servo/servo/pull/19573 ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Disallowing warnings on all crates?
On Fri, Dec 15, 2017 at 04:40:42PM +0100, Emilio Cobos Álvarez wrote: > Hi, > > Tigercosmos sent a patch today adding #![deny(warnings)] to a bunch of > crates[1]. > > Just wanted to give a heads-up / ask whether there's any objection to > the change before going ahead and r+ it. Does it sound reasonable to > everyone? It may make the servo-with-rust-nightly build fail a bit > earlier, but probably that's good, actually. >From a Firefox point of view, #![deny(warnings)] is already causing problems, and I don't think it's a good idea to have that in source, essentially for the same reason it's not a good idea to have -Werror set by default on C/C++ projects. Now, to be more specific about what problems we're already seeing with #![deny(warnings)], they are around the fact that when you bisect, the build fails on older code that triggers warnings that didn't exist in the stable rust compiler back when the code was written, but that do in the newer compiler, which you're using while bisecting. I'd actually go as far as saying that #![deny(warnings)] shouldn't even exist in rust at all for this reason, and CIs should be setting the equivalent to -Werror (which I think -D warnings is). Mike ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Disallowing warnings on all crates?
On 12/15/2017 05:47 PM, Mike Hommey wrote: > On Fri, Dec 15, 2017 at 04:40:42PM +0100, Emilio Cobos Álvarez wrote: >> Hi, >> >> Tigercosmos sent a patch today adding #![deny(warnings)] to a bunch of >> crates[1]. >> >> Just wanted to give a heads-up / ask whether there's any objection to >> the change before going ahead and r+ it. Does it sound reasonable to >> everyone? It may make the servo-with-rust-nightly build fail a bit >> earlier, but probably that's good, actually. > > From a Firefox point of view, #![deny(warnings)] is already causing > problems, and I don't think it's a good idea to have that in source, > essentially for the same reason it's not a good idea to have -Werror set > by default on C/C++ projects. > > Now, to be more specific about what problems we're already seeing with > #![deny(warnings)], they are around the fact that when you bisect, the > build fails on older code that triggers warnings that didn't exist in > the stable rust compiler back when the code was written, but that do in > the newer compiler, which you're using while bisecting. Rust supports a --cap-lints option I always use to bisect, fwiw, what'd be the problem with using that when bisecting? Servo actually fixes the rust compiler version and uses unstable features all over the place, so I think the tradeoffs are different here. > I'd actually go as far as saying that #![deny(warnings)] shouldn't even > exist in rust at all for this reason, and CIs should be setting the > equivalent to -Werror (which I think -D warnings is). > > Mike > ___ > dev-servo mailing list > dev-servo@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-servo > ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] Disallowing warnings on all crates?
Could we modify the pragma to include the maximum error number that is known to be supported, and make it a compiler policy for new warnings that might impact existing code to have monotonic numbering? On Dec 15, 2017 10:48, "Mike Hommey" wrote: > On Fri, Dec 15, 2017 at 04:40:42PM +0100, Emilio Cobos Álvarez wrote: > > Hi, > > > > Tigercosmos sent a patch today adding #![deny(warnings)] to a bunch of > > crates[1]. > > > > Just wanted to give a heads-up / ask whether there's any objection to > > the change before going ahead and r+ it. Does it sound reasonable to > > everyone? It may make the servo-with-rust-nightly build fail a bit > > earlier, but probably that's good, actually. > > From a Firefox point of view, #![deny(warnings)] is already causing > problems, and I don't think it's a good idea to have that in source, > essentially for the same reason it's not a good idea to have -Werror set > by default on C/C++ projects. > > Now, to be more specific about what problems we're already seeing with > #![deny(warnings)], they are around the fact that when you bisect, the > build fails on older code that triggers warnings that didn't exist in > the stable rust compiler back when the code was written, but that do in > the newer compiler, which you're using while bisecting. > > I'd actually go as far as saying that #![deny(warnings)] shouldn't even > exist in rust at all for this reason, and CIs should be setting the > equivalent to -Werror (which I think -D warnings is). > > Mike > ___ > dev-servo mailing list > dev-servo@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-servo > ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
