The default bug view has changed. See this FAQ.

Makes content/html/content/src warning free

RESOLVED FIXED in mozilla12

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
mozilla12
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 586773 [details] [diff] [review]
Patch v1
Attachment #586773 - Flags: review?(bzbarsky)
Do you just mean free of GCC warnings?
(Assignee)

Comment 2

5 years ago
(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 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?
Attachment #586773 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 4

5 years ago
(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.
https://hg.mozilla.org/mozilla-central/rev/8e12454829d3
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [needs review]
Target Milestone: --- → mozilla12
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).
(In reply to Daniel Holbert [:dholbert] from comment #6)
> The first are relatively easy to fix and have patches.
er "the first two"
(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.)
You need to log in before you can comment on or make changes to this bug.