Win64 compiler warnings in nsTSubstring.h (warning C4267: 'argument' : conversion from 'size_t' to XXX)

RESOLVED FIXED in mozilla31

Status

()

RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: RyanVM, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
mozilla31
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
These show up in Win64 builds and spam the console incessantly.

nsTSubstring.h(359) : warning C4267: 'argument' : conversion from 'size_t' to 'nsAString_internal::size_type', possible loss of data
nsTSubstring.h(363) : warning C4267: 'argument' : conversion from 'size_t' to 'nsAString_internal::size_type', possible loss of data

nsCOMArray.h(198) : warning C4267: 'argument' : conversion from 'size_t' to 'int32_t', possible loss of data
(Reporter)

Comment 1

5 years ago
nsTSubstring.h

357      void NS_FASTCALL AssignASCII( const char* data )
358        {
359          AssignASCII(data, strlen(data));
360        }
361      bool NS_FASTCALL AssignASCII( const char* data, const fallible_t& ) NS_WARN_UNUSED_RESULT
362        {
363          return AssignASCII(data, strlen(data), fallible_t());
364        }

nsCOMArray.h

189 inline void
190 ImplCycleCollectionTraverse(nsCycleCollectionTraversalCallback& aCallback,
191                             nsCOMArray_base& aField,
192                             const char* aName,
193                             uint32_t aFlags = 0)
194 {
195     aFlags |= CycleCollectionEdgeNameArrayFlag;
196     size_t length = aField.Count();
197     for (size_t i = 0; i < length; ++i) {
198         CycleCollectionNoteChild(aCallback, aField[i], aName, aFlags);
199     }
200 }
The problem for nsCOMArray is that the type of operator[] is this:
  nsISupports* operator[](int32_t aIndex)
That seems kind of bad.

But anyways, changing the size_t type of length and i to int32_t should fix this.  I'd guess there's a similar problem for the other ImplCycleCollectionTraverse functions for arrays.
I split off the nsCOMArray stuff into bug 947626.
Summary: Win64 compiler warnings in nsTSubstring.h and nsCOMArray.h (warning C4267: 'argument' : conversion from 'size_t' to XXX) → Win64 compiler warnings in nsTSubstring.h (warning C4267: 'argument' : conversion from 'size_t' to XXX)
Created attachment 8412637 [details] [diff] [review]
avoid warnings from using size_t where it should be int32_t
Comment on attachment 8412637 [details] [diff] [review]
avoid warnings from using size_t where it should be int32_t

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

::: xpcom/glue/nsTObserverArray.h
@@ +386,5 @@
>                              uint32_t aFlags = 0)
>  {
>    aFlags |= CycleCollectionEdgeNameArrayFlag;
> +  int32_t length = aField.Length();
> +  for (int32_t i = 0; i < length; ++i) {

Shouldn't these be uint32_t, since Length() returns nsAutoTObserverArray::size_type (which AFAICS is unfortunately not public)?
Comment on attachment 8412637 [details] [diff] [review]
avoid warnings from using size_t where it should be int32_t

Similar to the nsCOMArray changes
Attachment #8412637 - Flags: review?(ehsan)
Created attachment 8412641 [details] [diff] [review]
fix spammy implicit type-conversion warnings in nsTSubstring.h

The SafeCast stuff is really only for 64-bit builds, and I'm not sure it's
completely worthwhile, but hey...
Attachment #8412641 - Flags: review?(ehsan)

Comment 8

4 years ago
Comment on attachment 8412637 [details] [diff] [review]
avoid warnings from using size_t where it should be int32_t

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

r=me with the below fixed!

::: xpcom/glue/nsTArray.h
@@ +1643,5 @@
>                              const char* aName,
>                              uint32_t aFlags = 0)
>  {
>    aFlags |= CycleCollectionEdgeNameArrayFlag;
> +  int32_t length = aField.Length();

nsTArray::size_type is uint32_t.  Please use either one here, not int32_t.

::: xpcom/glue/nsTObserverArray.h
@@ +386,5 @@
>                              uint32_t aFlags = 0)
>  {
>    aFlags |= CycleCollectionEdgeNameArrayFlag;
> +  int32_t length = aField.Length();
> +  for (int32_t i = 0; i < length; ++i) {

This too.
Attachment #8412637 - Flags: review?(ehsan) → review+

Updated

4 years ago
Attachment #8412641 - Flags: review?(ehsan) → review+
Created attachment 8413379 [details] [diff] [review]
avoid warnings from using size_t in ImplCycleCollectionTraverse()

Sometimes I hate C++ & templates.... Done the simpler way ehsan asked for instead.  https://tbpl.mozilla.org/?tree=Try&rev=409243389463
(Reporter)

Comment 13

4 years ago
https://hg.mozilla.org/mozilla-central/rev/f656d5bc051e
https://hg.mozilla.org/mozilla-central/rev/e248f673c2f2
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Blocks: 1072071
You need to log in before you can comment on or make changes to this bug.