Re: Improving visibility of compiler warnings

On 5/25/17 6:11 PM, Eric Rahm wrote: I think we disable it for local builds because we don't control what versions of tools folks use. So clang vFOO might spit out errors we don't see in clang vBAR and it would be a huge pain if those failed locally even though they'd be fine in automation. It

Re: Improving visibility of compiler warnings

over the years about this. What specifically motivated me to make this change recently was finding a pattern of feature requests during bug triage that all seemed to point to improving visibility of compiler warnings as a potential solution. I fully acknowledge that the spew of warnings a

Re: Improving visibility of compiler warnings

On Thu, May 25, 2017 at 4:11 PM, Eric Rahm wrote: > I think we disable it for local builds because we don't control what > versions of tools folks use. So clang vFOO might spit out errors we don't > see in clang vBAR and it would be a huge pain if those failed locally even >

Re: Improving visibility of compiler warnings

On Friday, May 26, 2017 at 11:08:09 AM UTC+12, Andrew McCreight wrote: > On Thu, May 25, 2017 at 4:03 PM, wrote: > > > On Friday, May 26, 2017 at 6:03:02 AM UTC+12, Chris Peterson wrote: > > > On 2017-05-25 5:31 AM, Ehsan Akhgari wrote: > > > > On 05/19/2017 02:44 PM,

Re: Improving visibility of compiler warnings

I think we disable it for local builds because we don't control what versions of tools folks use. So clang vFOO might spit out errors we don't see in clang vBAR and it would be a huge pain if those failed locally even though they'd be fine in automation. -e On Thu, May 25, 2017 at 4:07 PM,

Re: Improving visibility of compiler warnings

On Thu, May 25, 2017 at 4:03 PM, wrote: > On Friday, May 26, 2017 at 6:03:02 AM UTC+12, Chris Peterson wrote: > > On 2017-05-25 5:31 AM, Ehsan Akhgari wrote: > > > On 05/19/2017 02:44 PM, Gregory Szorc wrote: > > >> `mach build` attempts to parse compiler warnings to a

Re: Improving visibility of compiler warnings

On Friday, May 26, 2017 at 6:03:02 AM UTC+12, Chris Peterson wrote: > On 2017-05-25 5:31 AM, Ehsan Akhgari wrote: > > On 05/19/2017 02:44 PM, Gregory Szorc wrote: > >> `mach build` attempts to parse compiler warnings to a persisted > >> "database." > >> You can view a list of compiler warnings

Re: Improving visibility of compiler warnings

On Thu, May 25, 2017 at 08:31:24AM -0400, Ehsan Akhgari wrote: What was the motivation behind this change? Was there a complaint from a significant number of developers about it being difficult fixing compiler warnings grepping for things like "warning:" or using ./mach warnings-list? Was the

Re: Improving visibility of compiler warnings

On 2017-05-25 5:31 AM, Ehsan Akhgari wrote: On 05/19/2017 02:44 PM, Gregory Szorc wrote: `mach build` attempts to parse compiler warnings to a persisted "database." You can view a list of compiler warnings post build by running `mach warnings-list`. The intent behind this feature was to make it

Re: Improving visibility of compiler warnings

I'd like to second Ehsan's point, but also expand upon it into a more general observation. As it becomes progressively more difficult to build Firefox without mach, it becomes increasingly important that mach respect people's workflows. For those of us who were comfortable with make and the

Re: Improving visibility of compiler warnings

On 5/25/17 8:31 AM, Ehsan Akhgari wrote: This currently only serves to make it more difficult to find compiler errors when they occur. Fwiw (and not to distract from your main point), https://bugzilla.mozilla.org/show_bug.cgi?id=1367405 just got fixed so we should no longer get the warning

Re: Improving visibility of compiler warnings

On 05/19/2017 02:44 PM, Gregory Szorc wrote: `mach build` attempts to parse compiler warnings to a persisted "database." You can view a list of compiler warnings post build by running `mach warnings-list`. The intent behind this feature was to make it easier to find and fix compiler warnings.

Re: Improving visibility of compiler warnings

On Thu, May 25, 2017 at 6:03 AM, Nathan Froyd wrote: > Where does NSS do this? Cloning the NSS tree and grepping for > sign-compare turns up no disabling of -Wsign-compare, except perhaps > in XCode for gtest itself. My bad, -Wsign-compare is in -Wextra, and we don't enable

Re: Improving visibility of compiler warnings

I had it pointed out to me on IRC by jcristau that `chronic` already exists as a Unix utility from moreutils: https://manpages.debian.org/jessie/moreutils/chronic.1.en.html It does pretty much the exact same thing as nowarn, but is probably better written ^.^. On Tue, May 23, 2017 at 11:56 AM,

Re: Improving visibility of compiler warnings

I don't have enough time to work on a proper solution, but I wrote a simple rust wrapper which just consumes stderr from the wrapped process, and doesn't write it out unless the internal command exited with a non-zero exit code. I figured I'd post it in case anyone else finds it useful.

Re: Improving visibility of compiler warnings

On Sat, 20 May 2017 14:59:11 -0400, Eric Rescorla wrote: > On Sat, May 20, 2017 at 1:16 PM, Kris Maglione > wrote: > >> On Sat, May 20, 2017 at 08:36:13PM +1000, Martin Thomson wrote: >> >>> Hmm, these are all -Wsign-compare issues bar one, which is fixed >>> upstream. We

Re: Improving visibility of compiler warnings

Hi, On 2017/05/20 19:36, Martin Thomson wrote: On Sat, May 20, 2017 at 4:55 AM, Kris Maglione wrote: Can we make some effort to get clean warnings output at the end of standard builds? A huge chunk of the warnings come from NSS and NSPR, and should be easily fixable.

Re: Improving visibility of compiler warnings

On Sat, May 20, 2017 at 1:16 PM, Kris Maglione wrote: > On Sat, May 20, 2017 at 08:36:13PM +1000, Martin Thomson wrote: > >> On Sat, May 20, 2017 at 4:55 AM, Kris Maglione >> wrote: >> >>> Can we make some effort to get clean warnings output at the

Re: Improving visibility of compiler warnings

On Sat, May 20, 2017 at 08:36:13PM +1000, Martin Thomson wrote: On Sat, May 20, 2017 at 4:55 AM, Kris Maglione wrote: Can we make some effort to get clean warnings output at the end of standard builds? A huge chunk of the warnings come from NSS and NSPR, and should be

Re: Improving visibility of compiler warnings

On Sat, May 20, 2017 at 4:55 AM, Kris Maglione wrote: > Can we make some effort to get clean warnings output at the end of standard > builds? A huge chunk of the warnings come from NSS and NSPR, and should be > easily fixable. Hmm, these are all -Wsign-compare issues bar

Re: Improving visibility of compiler warnings

On Fri, May 19, 2017 at 1:27 PM, Michael Layzell wrote: > With regard to ALLOW_COMPILER_WARNINGS, I'm strongly of the opinion that we > should be providing an option to hide all warnings from modules marked as > ALLOW_COMPILER_WARNINGS now, as they are only in "external"

Re: Improving visibility of compiler warnings

I strongly agree with you, Michael! Especially now that I'm using IceCC, I pretty much always have to use search-and-replace to find compiler errors. It's a pointless drain on productivity. On Fri, May 19, 2017 at 1:27 PM, Michael Layzell wrote: > With regard to

Re: Improving visibility of compiler warnings

While I agree with that in general, it seems like the warnings in NSS and NSPR, at least, are our responsibility, and should be fixed. I notice the huge number of warnings scrolling by from NSS, in particular, every time I compile, and they make me worry. On Fri, May 19, 2017 at 04:27:47PM

Re: Improving visibility of compiler warnings

With regard to ALLOW_COMPILER_WARNINGS, I'm strongly of the opinion that we should be providing an option to hide all warnings from modules marked as ALLOW_COMPILER_WARNINGS now, as they are only in "external" libraries which most of us are not working on, and they really clog up my compiler

Re: Improving visibility of compiler warnings

It is great to see a good use for compiler warnings and alerts. We have added a lot of data to perfherder and the build metrics are not covered by any sheriffs by default. For these if it is clear who introduced them, we will comment in the bug as a note, but there are no intentions to file

Re: Improving visibility of compiler warnings

On 2017-05-19 2:44 PM, Gregory Szorc wrote: The Perfherder alerts and tracking are informational only: nobody will be backing you out merely because you added a compiler warning. While the possibility of doing that now exists, I view that decision as up to the C++ module. I'm not going to

Re: Improving visibility of compiler warnings

I'm not sure if this is exactly the same as the ALLOW_COMPILER_WARNINGS that we use for warnings-on-errors, but FWIW as of a couple of months ago I cleaned out the last warning-allowance in our "own" code. ALLOW_COMPILER_WARNINGS usage is now only in external libraries (I'm counting NSS and NSPR

Re: Improving visibility of compiler warnings

I've actually been meaning to post about this, with some questions. I definitely like that we now print warnings at the end of builds, since it makes them harder to ignore. But the current amount of warnings spew at the end of every build is problematic, especially when a build fails and I

Improving visibility of compiler warnings

`mach build` attempts to parse compiler warnings to a persisted "database." You can view a list of compiler warnings post build by running `mach warnings-list`. The intent behind this feature was to make it easier to find and fix compiler warnings. After all, something out of sight is out of mind.