dist\include\jsapi.h(3526) : warning C4267: 'argument' : conversion from 'size_t' to 'jsint', possible loss of data

RESOLVED FIXED in mozilla33

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

Trunk
mozilla33
x86_64
Windows Vista
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
There is a lot of warning for jsapi.h on Win64 build...
(Assignee)

Updated

7 years ago
Attachment #603173 - Flags: review?(luke)
I don't quite understand: JSIdArray::length is an integer as well as the 'index' parameter of JS_IdArrayGet.  How does changing 'index' to a size_t fix the warnings?
(Assignee)

Comment 3

7 years ago
http://mxr.mozilla.org/mozilla-central/source/js/src/jsapi.h?mark=3522-3526#3522

3522     jsid operator[](size_t i) const {
3523         JS_ASSERT(idArray);
3524         JS_ASSERT(i < length());
3525         return JS_IdArrayGet(context, idArray, i);
3526     }

i is size_t, but 3rd parameter of JS_IdArrayGet is int.  So VC++ shows warning.  Should I cast i to (unsigned)i instead of?
(Assignee)

Comment 4

7 years ago
s/(unsigned)i/(int)i
How about just changing 'index' and 'length' to size_t so its size_t all around?
(Assignee)

Comment 6

6 years ago
(In reply to Luke Wagner [:luke] from comment #5)
> How about just changing 'index' and 'length' to size_t so its size_t all
> around?

I don't know that we should allow over 4GB area for 64-bit.  But if supporting it, we should use size_t on all for size.

At least, 3rd parameter of JS_IdArrayGet should use unsigned value since negative value will be asserted.
Yes, we do seem to use 'unsigned' for length.  How about changing the types to 'unsigned' then?
(Assignee)

Updated

6 years ago
Attachment #603173 - Flags: review?(luke)
(Assignee)

Comment 8

6 years ago
Created attachment 606164 [details] [diff] [review]
fix v2
Attachment #603173 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Attachment #606164 - Flags: review?(luke)
Comment on attachment 606164 [details] [diff] [review]
fix v2

Could you also change JSIdArray::length to unsigned (thus removing the need for casts in JS_IdArrayGet).
Attachment #606164 - Flags: review?(luke) → review+
(Assignee)

Comment 10

6 years ago
(In reply to Luke Wagner [:luke] from comment #9)
> Comment on attachment 606164 [details] [diff] [review]
> fix v2
> 
> Could you also change JSIdArray::length to unsigned (thus removing the need
> for casts in JS_IdArrayGet).

ida->length is int type.  should I change it?  If so, we need a lot of fix.

If changing JSIdArray::length to unsigned instead of size_t, I need this cast since this is int vs unsigned.
Ok, nevermind then.
Makoto: did you forget to land this r+'d patch from 2012? :) The patch still looks relevant.
Status: NEW → ASSIGNED
Flags: needinfo?(m_kato)
(Assignee)

Comment 13

4 years ago
(In reply to Chris Peterson (:cpeterson) from comment #12)
> Makoto: did you forget to land this r+'d patch from 2012? :) The patch still
> looks relevant.

Ah, I will land this.
Flags: needinfo?(m_kato)
https://hg.mozilla.org/mozilla-central/rev/3087b33b8037
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33

Updated

4 years ago
Duplicate of this bug: 940330
You need to log in before you can comment on or make changes to this bug.