716338 - Makes content/html/content/src warning free

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

Comment 1

[Nicholas Nethercote [inactive]]

Do you just mean free of GCC warnings?

Comment 2

[Mounir Lamouri (:mounir)]

(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 [:bzbarsky]]

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)]

(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] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/8e12454829d3

Comment 6

[Daniel Holbert [:dholbert]]

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]]

(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]]

(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.)