warning C4244: '=' : conversion from 'PRTime' to 'PRUint32', possible loss of data

RESOLVED WORKSFORME

Status

()

Core
Widget
RESOLVED WORKSFORME
11 years ago
7 years ago

People

(Reporter: mats, Assigned: mats)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [build_warning])

(Assignee)

Description

11 years ago
When compiling this line:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/events/src/nsDOMXULCommandEvent.cpp&rev=1.7&root=/cvsroot&mark=52#41

I get this warning (Windows XP, Visual compiler):
c:/moz/firefox/mozilla/content/events/src/nsDOMXULCommandEvent.cpp(52) : warning C4244: '=' : conversion from 'PRTime' to 'PRUint32', possible loss of data

Because PR_Now() returns PRTime (==PRInt64) and nsEvent::time is PRUint32.

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/widget/public/nsGUIEvent.h&rev=3.145&root=/cvsroot&mark=386-388#356
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/nsprpub/pr/include/prtime.h&rev=3.10&root=/cvsroot#164

(also, the comment in nsEvent looks incorrect -- it doesn't match the
comment for PR_Now() anyway)

Can we change the type for nsEvent::time to PRTime?
There's a known mess with nsEvent::time and what it _should_ be.  Per DOM3 Events spec, this member should in fact be in milliseconds, last I checked.  See http://www.w3.org/TR/2003/NOTE-DOM-Level-3-Events-20031107/events.html#Events-Event-timeStamp

None of which is to say we might not want a 64-bit value here so we can have this working past 2038.  But the point is that assigning PR_Now() into it (as we do in all sorts of places, including nsDOMEvent::nsDOMevent) is bogus.

Note that I'm sure we have bugs covering this...
QA Contact: general

Comment 2

8 years ago
(In reply to comment #1)
> There's a known mess with nsEvent::time and what it _should_ be.  Per DOM3
> Events spec, this member should in fact be in milliseconds, last I checked. 
> See
> http://www.w3.org/TR/2003/NOTE-DOM-Level-3-Events-20031107/events.html#Events-Event-timeStamp
> 
> None of which is to say we might not want a 64-bit value here so we can have
> this working past 2038.  But the point is that assigning PR_Now() into it (as
> we do in all sorts of places, including nsDOMEvent::nsDOMevent) is bogus.
> 
> Note that I'm sure we have bugs covering this...

as in, the non-perf aspects of bug 58310?

Updated

7 years ago
Whiteboard: [build_warning]

Updated

7 years ago
Blocks: 187528

Comment 3

7 years ago
There is no such warning in nsDOMXULCommandEvent.cpp (in current code base). Also verified that no warning is there in nighly logs of mac, windows and linux (debug also).
Status: UNCONFIRMED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.