1124029 - Fix some warnings when build for win64 with --enable-warnings-as-error

Status: RESOLVED FIXED

(Core :: General, defect)

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

Attachment: patch 1 - Fix printf type warning on AccessibleWrap

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

Attachment: patch 2 - Fix 64bit shift warning in imgFrame

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

Attachment: patch 3 - Fix unsigned/signed mismatch in MathAlgorithms

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

Attachment: patch 4 - Suppress different __unaligned qualifiers warnings

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #4)
> Created attachment 8552173 [details] [diff] [review]
> patch 4 - Suppress different __unaligned qualifiers warnings

This patch fixes warnings like:
Warning: C4090 in xpcom\io\nsLocalFileWin.cpp: 'initializing' : different '__unaligned' qualifiers
xpcom/io/nsLocalFileWin.cpp(188) : warning C4090: 'initializing' : different '__unaligned' qualifiers

Comment 6

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8552171 [details] [diff] [review]
patch 2 - Fix 64bit shift warning in imgFrame

What's the warning/error that we're fixing here?

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Timothy Nikkel (:tn) from comment #6)
> Comment on attachment 8552171 [details] [diff] [review]
> patch 2 - Fix 64bit shift warning in imgFrame
> 
> What's the warning/error that we're fixing here?

Warning: C4334 in image\src\imgFrame.h: '<<' : result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?)
image\src\imgFrame.h(299) : warning C4334: '<<' : result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?)

Comment 8

[[e-mail]]

Comment on attachment 8552171 [details] [diff] [review]
patch 2 - Fix 64bit shift warning in imgFrame

Seeing as the return value is uint32_t, we don't need a 64-bit shift, so why not cast the sizeof to uint32_t instead?

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

(In reply to [e-mail] from comment #8)
> Comment on attachment 8552171 [details] [diff] [review]
> patch 2 - Fix 64bit shift warning in imgFrame
> 
> Seeing as the return value is uint32_t, we don't need a 64-bit shift, so why
> not cast the sizeof to uint32_t instead?

Because according to the context, I think size_t should be the correct type here. I wanted to also change the return type of that function, but I guess it is probably too much for just fixing a warning.

Comment 10

[Xidorn Quan [:xidorn] UTC+10]

try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=862b786230e4

with another bug together

Comment 11

[Xidorn Quan [:xidorn] UTC+10]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6785604f4cc8
https://hg.mozilla.org/integration/mozilla-inbound/rev/7315bdbef259

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6785604f4cc8
https://hg.mozilla.org/mozilla-central/rev/7315bdbef259

Comment 13

[Benjamin Smedberg]

Comment on attachment 8552173 [details] [diff] [review]
patch 4 - Suppress different __unaligned qualifiers warnings

I don't have enough context to know whether the warning is spurious or whether this is a good fix.

Comment 14

[Nathan Froyd [:froydnj]]

Comment on attachment 8552173 [details] [diff] [review]
patch 4 - Suppress different __unaligned qualifiers warnings

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

I couldn't figure out where the __unaligned qualifiers were coming from, since MSDN says ILCreateFromPath returns a PIDLIST_ABSOLUTE, not a PUIDLIST_ABSOLUTE:

https://msdn.microsoft.com/en-us/library/windows/desktop/dd378420%28v=vs.85%29.aspx
https://msdn.microsoft.com/en-us/library/windows/desktop/bb773321%28v=vs.85%29.aspx

But looking at <ShTypes.h>, it looks like if you don't define STRICT_TYPED_ITEMIDS (which actually seems like a good idea), PIDLIST_ABSOLUTE silently inherits an __unaligned qualifier from being defined as a synonym to LPITEMIDLIST.

Comment 15

[Xidorn Quan [:xidorn] UTC+10]

https://hg.mozilla.org/integration/mozilla-inbound/rev/adbc9eec0490
https://hg.mozilla.org/integration/mozilla-inbound/rev/47834f1829b9

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/adbc9eec0490
https://hg.mozilla.org/mozilla-central/rev/47834f1829b9

Comment 17

[Xidorn Quan [:xidorn] UTC+10]

Attachment: patch 5 - Fix 64bit shift warning in nsVoidArray

Fix the same warning as comment 7.

Comment 18

[Xidorn Quan [:xidorn] UTC+10]

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

Comment 19

[(Away)]

__unaligned is meaningless in the amd64 build; it's only relevant to the Itanium compiler. Hacking around the warning is fine.

Comment 20

[Carsten Book [:Tomcat]]

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