fix compiler warnings in accessible/

RESOLVED FIXED in mozilla13

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

(Blocks: 1 bug)

unspecified
mozilla13
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Just a couple of signed-vs-unsigned comparisons.
(Assignee)

Comment 1

5 years ago
Created attachment 597093 [details] [diff] [review]
patch

Changing the types of a few variables is sufficient to satisfy the compiler.
Attachment #597093 - Flags: review?(surkov.alexander)

Comment 2

5 years ago
Comment on attachment 597093 [details] [diff] [review]
patch

forward review to Andrzej
Attachment #597093 - Flags: review?(surkov.alexander) → review?(askalski)

Comment 3

5 years ago
Andrzej, ping

Comment 4

5 years ago
on it now.

Comment 5

5 years ago
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.
Attachment #597093 - Flags: review?(askalski)
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

5 years ago
Comment on attachment 597093 [details] [diff] [review]
patch

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

Misunderstood bug summary, it's + now.
Attachment #597093 - Flags: review+
(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

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

guys, please file follow-up bugs for your findings
https://hg.mozilla.org/mozilla-central/rev/c331db290cd8
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.