Last Comment Bug 727163 - fix compiler warnings in accessible/
: fix compiler warnings in accessible/
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla13
Assigned To: Nathan Froyd [:froydnj]
:
:
Mentors:
Depends on:
Blocks: buildwarning
  Show dependency treegraph
 
Reported: 2012-02-14 10:52 PST by Nathan Froyd [:froydnj]
Modified: 2012-02-20 10:26 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (2.05 KB, patch)
2012-02-14 10:54 PST, Nathan Froyd [:froydnj]
andrzej.j.skalski: review+
Details | Diff | Splinter Review

Description Nathan Froyd [:froydnj] 2012-02-14 10:52:45 PST
Just a couple of signed-vs-unsigned comparisons.
Comment 1 Nathan Froyd [:froydnj] 2012-02-14 10:54:11 PST
Created attachment 597093 [details] [diff] [review]
patch

Changing the types of a few variables is sufficient to satisfy the compiler.
Comment 2 alexander :surkov 2012-02-14 17:30:57 PST
Comment on attachment 597093 [details] [diff] [review]
patch

forward review to Andrzej
Comment 3 alexander :surkov 2012-02-16 23:52:03 PST
Andrzej, ping
Comment 4 Andrzej Skalski 2012-02-17 03:46:30 PST
on it now.
Comment 5 Andrzej Skalski 2012-02-17 06:23:21 PST
Comment on attachment 597093 [details] [diff] [review]
patch

Review of attachment 597093 [details] [diff] [review]:
-----------------------------------------------------------------

After applying this patch I still get some warnings, like:

mozilla-central/accessible/src/base/nsAccessible.cpp:2681:56: warning: comparison between signed and unsigned integer expressions
mozilla-central/accessible/src/base/nsAccessible.cpp:2711:50: warning: comparison between signed and unsigned integer expressions

and

../../../dist/include/nsIDOMXULSelectCntrlEl.h:33:60: warning: ‘virtual nsresult nsIDOMXULSelectControlElement::GetSelectedItem(nsIDOMXULSelectControlItemElement**)’ was hidden
../../../dist/include/nsIDOMXULMultSelectCntrlEl.h:73:60: warning:   by ‘virtual nsresult nsIDOMXULMultiSelectControlElement::GetSelectedItem(PRInt32, nsIDOMXULSelectControlItemElement**)’

You know how to solve the one with arithmetics. For the "was hidden" ones, I think they need to be suppressed with precompiler directives (if the code is considered correct, and I am not an expert).

filter out your compiler output with
make clean 1>/dev/null && make 2>&1 1> /dev/null | grep 'warning'
on obj-dir/accessible, or (alternatively) do make > /dev/null and read stderr from the screen.

If anyone knows the way to pass -Werror flag to compiler just for this one sub-directory, please let me know.
Comment 6 :Ms2ger (⌚ UTC+1/+2) 2012-02-17 06:31:06 PST
FAIL_ON_WARNINGS = 1

in the makefile. For the DOM warning, I had this patch lying around:

diff --git a/dom/interfaces/xul/nsIDOMXULMultSelectCntrlEl.idl b/dom/interfaces/xul/nsIDOMXULMultSelectCntrlEl.idl
--- a/dom/interfaces/xul/nsIDOMXULMultSelectCntrlEl.idl
+++ b/dom/interfaces/xul/nsIDOMXULMultSelectCntrlEl.idl
@@ -37,16 +37,19 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsIDOMXULSelectCntrlEl.idl"
 
 [scriptable, uuid(98c367ca-ac5b-493c-83e9-1b6a67aee57b)]
 interface nsIDOMXULMultiSelectControlElement : nsIDOMXULSelectControlElement
 {
+%{C++
+  using nsIDOMXULSelectControlElement::GetSelectedItem;
+%}
   attribute DOMString selType;
 
   attribute nsIDOMXULSelectControlItemElement currentItem;
   attribute long currentIndex;
 
   readonly attribute nsIDOMNodeList selectedItems;
   
   void addItemToSelection(in nsIDOMXULSelectControlItemElement item);
Comment 7 Andrzej Skalski 2012-02-17 06:53:12 PST
Comment on attachment 597093 [details] [diff] [review]
patch

Review of attachment 597093 [details] [diff] [review]:
-----------------------------------------------------------------

Misunderstood bug summary, it's + now.
Comment 8 Trevor Saunders (:tbsaunde) 2012-02-17 07:12:59 PST
(In reply to Ms2ger from comment #6)
> FAIL_ON_WARNINGS = 1
> 
> in the makefile. For the DOM warning, I had this patch lying around:

I'm not sure I actually want to do that, but anyway see the comment about other warnings.

> diff --git a/dom/interfaces/xul/nsIDOMXULMultSelectCntrlEl.idl
> b/dom/interfaces/xul/nsIDOMXULMultSelectCntrlEl.idl
> --- a/dom/interfaces/xul/nsIDOMXULMultSelectCntrlEl.idl
> +++ b/dom/interfaces/xul/nsIDOMXULMultSelectCntrlEl.idl
> @@ -37,16 +37,19 @@
>   *
>   * ***** END LICENSE BLOCK ***** */
>  
>  #include "nsIDOMXULSelectCntrlEl.idl"
>  
>  [scriptable, uuid(98c367ca-ac5b-493c-83e9-1b6a67aee57b)]
>  interface nsIDOMXULMultiSelectControlElement : nsIDOMXULSelectControlElement
>  {
> +%{C++
> +  using nsIDOMXULSelectControlElement::GetSelectedItem;
> +%}
>    attribute DOMString selType;
>  
>    attribute nsIDOMXULSelectControlItemElement currentItem;
>    attribute long currentIndex;
>  
>    readonly attribute nsIDOMNodeList selectedItems;
>    
>    void addItemToSelection(in nsIDOMXULSelectControlItemElement item);

if you fix that issue you'll be my hero for oh say an hour :)
Comment 9 alexander :surkov 2012-02-18 02:03:09 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/c331db290cd8

guys, please file follow-up bugs for your findings
Comment 10 Ed Morley [:emorley] 2012-02-20 10:26:34 PST
https://hg.mozilla.org/mozilla-central/rev/c331db290cd8

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