Last Comment Bug 716338 - Makes content/html/content/src warning free
: Makes content/html/content/src warning free
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla12
Assigned To: Mounir Lamouri (:mounir)
:
Mentors:
Depends on: 703121
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-08 02:23 PST by Mounir Lamouri (:mounir)
Modified: 2012-01-10 14:35 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (4.15 KB, patch)
2012-01-08 02:23 PST, Mounir Lamouri (:mounir)
bzbarsky: review+
Details | Diff | Splinter Review

Description Mounir Lamouri (:mounir) 2012-01-08 02:23:38 PST
Created attachment 586773 [details] [diff] [review]
Patch v1
Comment 1 Nicholas Nethercote [:njn] 2012-01-08 14:03:18 PST
Do you just mean free of GCC warnings?
Comment 2 Mounir Lamouri (:mounir) 2012-01-08 15:58:51 PST
(In reply to Nicholas Nethercote [:njn] from comment #1)
> Do you just mean free of GCC warnings?

Warning-free as in --enable-warnings-as-errors doesn't make the compilation to fail. So warning-free on our Linux and Mac build bots.
Comment 3 Boris Zbarsky [:bz] (TPAC) 2012-01-09 08:09:57 PST
Comment on attachment 586773 [details] [diff] [review]
Patch v1

This seems fine, though the NULL/nsnull thing is a bit worrying.  Why was that an issue?
Comment 4 Mounir Lamouri (:mounir) 2012-01-09 09:18:38 PST
(In reply to Boris Zbarsky (:bz) from comment #3)
> Comment on attachment 586773 [details] [diff] [review]
> Patch v1
> 
> This seems fine, though the NULL/nsnull thing is a bit worrying.  Why was
> that an issue?

In file included from ../../../../dist/include/nsRefPtrHashtable.h:42:0,
                 from /home/volkmar/projects/mozilla/gecko/content/html/content/src/../../../base/src/nsDOMAttributeMap.h:48,
                 from /home/volkmar/projects/mozilla/gecko/content/html/content/src/../../../base/src/nsGenericElement.h:56,
                 from /home/volkmar/projects/mozilla/gecko/content/html/content/src/../../../base/src/nsStyledElement.h:50,
                 from /home/volkmar/projects/mozilla/gecko/content/html/content/src/../../../base/src/nsMappedAttributeElement.h:48,
                 from /home/volkmar/projects/mozilla/gecko/content/html/content/src/nsGenericHTMLElement.h:41,
                 from /home/volkmar/projects/mozilla/gecko/content/html/content/src/nsHTMLFormElement.h:45,
                 from /home/volkmar/projects/mozilla/gecko/content/html/content/src/nsHTMLFormElement.cpp:37:
../../../../dist/include/nsBaseHashtable.h: In member function ‘UserDataType nsBaseHashtable<KeyClass, DataType, UserDataType>::Get(nsBaseHashtable<KeyClass, DataType, UserDataType>::KeyType) const [with KeyClass = nsStringCaseInsensitiveHashKey, DataType = unsigned int, UserDataType = unsigned int, nsBaseHashtable<KeyClass, DataType, UserDataType>::KeyType = const nsAString_internal&]’:
/home/volkmar/projects/mozilla/gecko/content/html/content/src/nsHTMLFormElement.cpp:2068:72:   instantiated from here
../../../../dist/include/nsBaseHashtable.h:150:14: warning: converting to non-pointer type ‘unsigned int’ from NULL

The reason is because we want an |unsigned int| for this template instance. |nsnull| solves the warning and should not regress the situation where |UserDataType| is a pointer.
However, this is a bit hacky and the best way to solve that would probably to have a |Void()| method a la nsAuteRef.
Comment 5 Marco Bonardo [::mak] 2012-01-10 01:26:24 PST
https://hg.mozilla.org/mozilla-central/rev/8e12454829d3
Comment 6 Daniel Holbert [:dholbert] 2012-01-10 14:32:26 PST
FWIW, this triggered 5 distinct build errors for me on GCC 4.6 x86_64 (or rather turned 5 distinct GCC 4.6 warnings into errors):
 Bug 717004
 Bug 717015
 Bug 679832 (see bug 679832 comment 8)
 Bug 717025
 Bug 717034

The first are relatively easy to fix and have patches.  The latter 3 aren't so straightforward, though...  For the record, all of these except for bug 679832 are "unused"/"unused-but-set-variable" for nsresult return values that are captured but ignored.  (which is a "useful" type of warning -- they indicate that we're ignoring possible failure conditions.)

I guess I'll just add hackarounds or build without --enable-warnings-as-errors for now, but I wonder whether it'd be worth backing this out until this directory is GCC-4.6-warning-free.  I suspect that's the version of GCC that most Linux developers are using (or will be using as of their next OS upgrade).
Comment 7 Daniel Holbert [:dholbert] 2012-01-10 14:33:18 PST
(In reply to Daniel Holbert [:dholbert] from comment #6)
> The first are relatively easy to fix and have patches.
er "the first two"
Comment 8 Daniel Holbert [:dholbert] 2012-01-10 14:35:56 PST
(Ah, looks like Mats has patches for the last two over in bug 716904; nice. So that just leaves the middle one (Bug 679832) which has a workaround attached that works on linux, but no good cross-platform fix as of yet.)

Note You need to log in before you can comment on or make changes to this bug.