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 <mich...@thelayzells.com> 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" libraries which > most of us are not working on, and they really clog up my compiler output > and make me have to scroll up many many pages in order to see the build > errors which are actually my fault. (see this bug: > https://bugzilla.mozilla.org/show_bug.cgi?id=1301688). Unfortunately, I > don't know enough about the build system to write a patch to do this. > > On Fri, May 19, 2017 at 2:59 PM, David Major <dma...@mozilla.com> wrote: > > > 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 as "external" for this purpose since I can't land > > code to m-c directly). > > > > On Fri, May 19, 2017, at 02:55 PM, Kris Maglione wrote: > > > 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 > > > need > > > to scroll up several pages to find out why. > > > > > > 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. Most of the rest seem to come from vendored libraries, > > > and > > > should probably just be whitelisted if we can't get fixes upstreamed. > > > > > > On Fri, May 19, 2017 at 11:44:08AM -0700, 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. After all, something out of sight is > > out of > > > >mind. > > > > > > > >There have been a few recent changes to increase the visibility of > > compiler > > > >warnings with the expectation being that raising visibility will > > increase > > > >the chances of someone addressing them. After all, a compiler warning > is > > > >either a) valid and should be fixed or b) invalid and should be > ignored. > > > >Either way, a compiler warning shouldn't exist. > > > > > > > >First, `mach build` now prints a list of all parsed compiler warnings > at > > > >the end of the build. More details are in the commit message: > > > >https://hg.mozilla.org/mozilla-central/rev/4abe611696ab > > > > > > > >Second, Perfherder now records the compiler warning count as a metric. > > This > > > >means we now have alerts when compiler warnings are added or removed, > > like > > > >[1]. And, we can easily see graphs of compiler warnings over time, > like > > > >[2]. The compiler warnings are also surfaced in the "performance" tab > of > > > >build jobs on Treeherder, like [3]. > > > > > > > >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 advocate for it. So if you feel passionately, > > take > > > >it up with them :) > > > > > > > >Astute link clickers will note that the count metrics in CI are often > > noisy > > > >- commonly fluctuating by a value of 1-2. I suspect there are race > > > >conditions with interleaved process output and/or bugs in the compiler > > > >warnings parser/aggregator. Since the values are currently advisory > > only, > > > >I'm very much taking a "perfect is the enemy of good" attitude and > have > > no > > > >plans to improve the robustness. > > > > > > > >[1] https://treeherder.mozilla.org/perf.html#/alerts?id=6700 > > > >[2] https://mzl.la/2qFN0me and https://mzl.la/2rAkWR5 > > > >[3] > > > >https://treeherder.mozilla.org/#/jobs?repo=autoland&; > > selectedJob=100509284 > > > >_______________________________________________ > > > >dev-platform mailing list > > > >dev-platform@lists.mozilla.org > > > >https://lists.mozilla.org/listinfo/dev-platform > > > > > > -- > > > Kris Maglione > > > Senior Firefox Add-ons Engineer > > > Mozilla Corporation > > > > > > The X server has to be the biggest program I've ever seen that doesn't > > > do anything for you. > > > --Ken Thompson > > > > > > _______________________________________________ > > > dev-platform mailing list > > > dev-platform@lists.mozilla.org > > > https://lists.mozilla.org/listinfo/dev-platform > > _______________________________________________ > > dev-platform mailing list > > dev-platform@lists.mozilla.org > > https://lists.mozilla.org/listinfo/dev-platform > > > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
