727163 - fix compiler warnings in accessible/

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Nathan Froyd [:froydnj]]

Just a couple of signed-vs-unsigned comparisons.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: patch

Changing the types of a few variables is sufficient to satisfy the compiler.

Comment 2

[alexander :surkov (:asurkov)]

Comment on attachment 597093 [details] [diff] [review]
patch

forward review to Andrzej

Comment 3

[alexander :surkov (:asurkov)]

Andrzej, ping

Comment 4

[Andrzej Skalski]

on it now.

Comment 5

[Andrzej Skalski]

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 (he/him; ⌚ UTC+1/+2)]

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]

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

(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 (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c331db290cd8

guys, please file follow-up bugs for your findings

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c331db290cd8