Open
Bug 187528
(buildwarning)
Opened 22 years ago
Updated 10 months ago
[META] Fix compiler 'Build Warnings'
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
NEW
People
(Reporter: mozilla-bugs, Unassigned)
References
(Depends on 28 open bugs)
Details
(Keywords: meta)
There were some duplicated efforts (see bug 179775) with different people creating conflicting patches aiming at reducing the number of warning in Mozilla, so a general anti-warning tracking bug might be useful. Right now brad TBox lists 1269 warning. If we count duplicates (a header file warning replicated for every C/C++ file that include the header) only once, there are about 688 warnings. The current distribution by "kind" is roughly following: 184 Unused variable `...' 125 `...' might be used uninitialized in this function 62 `...' defined but not used 58 Choosing `...' over `...' For conversion from `...' to `...' Because conversion sequence for the argument is better 54 Comparison between signed and unsigned 44 ...was hidden by `...' 32 ANSI C forbids braced-groups within expressions 14 Return of negative value `...' to `...' Negative integer implicitly converted to unsigned type 10 Multi-character character constant 8 Value computed is not used 8 Suggest parentheses around assignment used as truth value 8 `...' is usually a function 6 ANSI does not permit the keyword `...' 5 Will be re-ordered to match declaration order 5 Using synthesized `...' for copy assignment 5 Missing initializer (near initialization for `...') 5 Member initializers for `...' 5 Enumeration value `...' not handled in switch 5 Converting NULL to non-pointer type 5 Assignment of negative value `...' to `...' Negative integer implicitly converted to unsigned type 5 And `...' 4 `...' has virtual functions but non-virtual destructor 3 Assignment to `...' from `...' 2 `...' within comment 2 Where cfront would use `...' 2 Left shift count >= width of type 2 Label `...' defined but not used 2 Converting of negative value `...' to `...' Negative integer implicitly converted to unsigned type 1 ...was hidden by `...' ...was hidden by `...' 1 String constant runs past end of line 1 Precision used with `...' format 1 Passing arg 1 of `...' discards qualifiers from pointer target type 1 Negative integer implicitly converted to unsigned type 1 Long unsigned int format, PRUint32 arg (arg 3) 1 Initialization to `...' from `...' 1 Initialization of negative value `...' to `...' Negative integer implicitly converted to unsigned type 1 Ignoring pragma: 1 `...' declared `...' but never defined 1 Decimal integer constant is so large that it is unsigned 1 Control reaches end of non-void function `...' 1 Control reaches end of non-void function 1 Char format, void arg (arg 2) 1 Argument `...' might be clobbered by `...' or `...' 1 ANSI C does not support the `...' length modifier 1 ANSI C does not allow extra `...' outside of a function 1 Aggregate has a partly bracketed initializer P.S. To simplify things, for those classes of warnings that have their own tracking bugs (such as bug 59652 for "uninitialized" warnings and 179028 for "hides" warnings), I only list the tracking bug as a dependency (see the dependency tree if you want the complete list).
Reporter | ||
Updated•21 years ago
|
Comment 1•21 years ago
|
||
*** Bug 228464 has been marked as a duplicate of this bug. ***
Updated•21 years ago
|
Updated•21 years ago
|
Updated•21 years ago
|
Updated•21 years ago
|
Updated•21 years ago
|
Comment 2•14 years ago
|
||
Back in 2003 it was written: > Right now brad TBox lists 1269 warning. If we count duplicates (a header file > warning replicated for every C/C++ file that include the header) only once, > there are about 688 warnings. The current distribution by "kind" is roughly > following: After fixing about 100 warnings on my own, I was guided to this bug entry. Today there are 1676 on my own compilation of TB3 using comm-central under linux, and unique warnings are 847. There were about 50-60 warnings of the nature as follows. These are newcomers which were not in the list above. $MOZSRC/mozilla/editor/libeditor/html/nsHTMLEditor.cpp:5878:25: warning: invoking macro NS_ENSURE_SUCCESS argument 2: empty macro arguments are undefined in ISO C90 and ISO C++98 $MOZSRC/mozilla/editor/libeditor/html/nsHTMLEditor.cpp:5879:32: warning: invoking macro NS_ENSURE_TRUE argument 2: empty macro arguments are undefined in ISO C90 and ISO C++98 These have been fixed and be posted to either Bug 609210 or by creating additional bug entry. (I added 609210 in the Depends list.)
Depends on: 609210
Comment 3•13 years ago
|
||
Apologies for the bugspam to all the newly added dependant bugs, but I feel that the current level of build warnings (eg 3100+ lines on win debug m-c tip, let alone other platforms) is beyond belief, so am starting on a crusade to eliminate them. Not only do the sheer number of existing warnings make it hard for contributors to work out if a patch has added more, but a proportion of them are potentially hiding real bugs (eg bug 659234). Once the count is down to a more sensible number, I'm hoping that bugs like bug 187015, bug 604850 or bug 513503 will be able to keep them more under control - and mean that people are no longer able to unknowingly add a bunch of warnings with each commit, instead having to fix them as they go.
Severity: trivial → normal
Depends on: 35544, 215544, 221126, 230001, 239445, 243018, 243763, 301985, 301988, 302152, 323916, 326171, 326315, 330154, 331301, 331316, 362799, 368159, 370415, 388110, 389320, 395786, 399700, 413981, 433515, 439144, 445414, 447838, 449291, 450163, 450918, 452331, 454489, 454761, 456150, 457803, 461099, 463199, 467092, 469749, 474612, 481282, 488991, 491190, 491191, 494686, 495156, 501614, 502904, 503862, 514150, 514152, 514153, 514154, 521329, 527006, 551082, 552788, 552793, 552821, 557952, 558163, 560266, 562403, 562503, 563191, 563226, 567862, 567868, 568692, 568870, 571769, 572412, 577471, 579392, 581467, 581468, 581473, 581474, 581476, 581477, 581478, 581481, 582427, 582443, 583607, 585414, 587496, 589022, 589032, 592207, 592425, 595995, 598049, 598051, 598058, 602044, 605174, 605180, 609188, 609431, 609710, 610228, 610809, 612777, 613984, 615608, 616788, 616792, 618427, 620510, 630290, 637179, 637605, 637607, 639865, 643173, 646115, 646160, 652398, 653295, 654287, 654618, 654891, 654893, 658081, 660922, 661962, 666605, 666610, 666612, 666614, 666615, 666617, 666621, 666627, 666629, 666632, 666638, 666641, 666646, 666651, 666655, 666656, 666659, 666661, 666664, 666670, 666672, 666676, 666677, 666679, 666688, 669236
Updated•13 years ago
|
Alias: buildwarning
Comment 4•13 years ago
|
||
In lieu of an automatic build warnings count on tinderbox/TBPL (a la bug 187015); for now I'm manually downloading build logs from various platforms (from try to ensure they get a clobber) and running through a script (based on one sent to me by Chiaki Ishikawa) to scan/summarise the warnings. The warning breakdown counts in the script are still a WIP (the compilers on each platform seem to like outputting the warnings in slightly different formats, so I'll need to work out the least false-positive regex), so for now, here's the summary. First figure is total build warning count, with the unique line count in parentheses. linux-debug: 1106 (700) linux-opt: 2785 (1023) linux64-debug: 1228 (785) linux64-opt: 3027 (1151) osx-debug: 647 (540) osx64-debug: 587 (483) osx64-opt: 2462 (1482) win-debug: 3149 (1660) win-opt: 3312 (1752) [Taken from a try job based on m-c 72331:821b5076d2c0 ~2011-07-05] I'll update this every now and again, to show the progress made by this meta.
Comment 5•11 years ago
|
||
I submitted a following comment https://bugzilla.mozilla.org/show_bug.cgi?id=563195#c43 --- begin quote --- > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #18) > > > At this point, I think I agree that it's not worth anybody's time to get > > this fully working. Something that I've wanted for a long time, though, is > > enabling the build system to tell you which warnings you have introduced > > compared to the previous build. I think that goes a long way to help using > > compiler warnings locally when writing code. > > Can someone introduce this to feature to TryBuilder for FF and TB? I submitted a bugzilla entry Bug 858543 - Creating "Warning Line Police" to reduce compile-time warning to discuss this. I think it is important for a long time health of the source base. TIA --- end quote --- Please check out Bug 858543 - Creating "Warning Line Police" to reduce compile-time warning I wonder if I should make 858543 block this bug entry? TIA
Updated•10 years ago
|
Depends on: Wuninitialized
Depends on: 1113210
Depends on: 1168895
Comment 6•8 years ago
|
||
We're now in a *much* better situation w.r.t. compiler warnings. The test machines run with --enable-warnings-as-errors and we use ALLOW_COMPILER_WARNINGS to opt out of warnings for third-party code on a directory basis, and we can also disable warnings for specific source files in moz.build files as necessary. So I suspect that most of the warnings cited in bugs blocking this bug either no longer occur, or occur in ways that we've explicitly allowed for (either third-party code, or one-off warnings in our code that we've decided to accept). The other bugs blocking this one that are about ways to reduce the warnings count (e.g. bug 858543) are likely no longer relevant. So I think there's a good chance that this bug can be closed. It might be good to have someone go through the still-open blocking bugs here and close them out (if appropriate). It's possible that this process might find evidence against my hypothesis that this bug can be closed. cpeterson, would you be interested in going through the open blockers? dholbert, do you have any opinions here?
Flags: needinfo?(dholbert)
Flags: needinfo?(cpeterson)
Comment 7•8 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #6) > cpeterson, would you be interested in going through the open blockers? Sounds good. I'll triage the open blockers soon. I agree that this particular bug is no longer useful.
Flags: needinfo?(cpeterson)
Comment 8•8 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #6) > dholbert, do you have any opinions here? tl;dr: I'm fine with this being closed, but I think we need a way to track these sorts of bugs going forward. (and for now, that's this bug) I agree that we've *largely* solved build warnings in our codebase (the main exception being imported 3rd-party code, which we can't manageably take spot-fixes for). However, I expect we will still have compile-warning bugs in our code in the future[1], and I think there's value in having a way of tracking them as a category. This metabug is not the best way of tracking them; perhaps we should add a new bugzilla keyword for build-warnings, and just use that instead of this metabug going forward? (and maybe someone can automate an update to all dependencies of this metabug to have that keyword) So: until we have a better alternative, in the rare case that I need to file a new build-warning bug, I'd probably still make it block this metabug. [1] While we've basically prevented developers from checking in new code that introduces build warnings (yay!), we could still have accidents where someone copypastes a moz.build file that happens to have ALLOW_COMPILER_WARNINGS into a new directory, and some build warnings slip through as a result. More legitimately, I fully expect that new gcc/clang/MSVC versions will create new categories of warnings (either enabled by default or useful-enough-that-we-want-to-enable-them), which will likely give us new build-warning-bugs that we'll want to file & fix (& track). (In reply to Chris Peterson [:cpeterson] from comment #7) > Sounds good. I'll triage the open blockers soon. Nice, thanks! You are a hero.
Flags: needinfo?(dholbert)
Updated•8 years ago
|
Depends on: Wunreachable-code
Depends on: 1242005
Updated•8 years ago
|
Component: Tracking → General
QA Contact: chofmann
Updated•8 years ago
|
Assignee: chofmann → nobody
Updated•7 years ago
|
No longer blocks: 1214953
Depends on: Wunused-member-function, 1214953
Updated•7 years ago
|
Depends on: Wunreachable-code-return
Updated•7 years ago
|
Depends on: Wduplicated-cond
Updated•6 years ago
|
Depends on: Wsuggest-override
Depends on: 1489944
Updated•5 years ago
|
Depends on: Wbitfield-enum-conversion
Updated•4 years ago
|
Depends on: Wempty-init-stmt
Updated•3 years ago
|
Depends on: -Wunused-parameter
Updated•2 years ago
|
Depends on: -Wunused-but-set-parameter
Updated•2 years ago
|
Depends on: Wlogical-op
Updated•2 years ago
|
Updated•2 years ago
|
Severity: normal → S3
Updated•1 year ago
|
Depends on: Wtautological-constant-in-range-compare
You need to log in
before you can comment on or make changes to this bug.
Description
•