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

RESOLVED FIXED in mozilla38

Status

()

Core
General
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

(Blocks: 1 bug)

Trunk
mozilla38
x86_64
Windows 8.1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

Comment hidden (empty)
(Assignee)

Comment 1

4 years ago
Created attachment 8552170 [details] [diff] [review]
patch 1 - Fix printf type warning on AccessibleWrap
Attachment #8552170 - Flags: review?(dbolter)
(Assignee)

Comment 2

4 years ago
Created attachment 8552171 [details] [diff] [review]
patch 2 - Fix 64bit shift warning in imgFrame
Attachment #8552171 - Flags: review?(tnikkel)
(Assignee)

Comment 3

4 years ago
Created attachment 8552172 [details] [diff] [review]
patch 3 - Fix unsigned/signed mismatch in MathAlgorithms
Attachment #8552172 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 4

4 years ago
Created attachment 8552173 [details] [diff] [review]
patch 4 - Suppress different __unaligned qualifiers warnings
Attachment #8552173 - Flags: review?(benjamin)
(Assignee)

Comment 5

4 years ago
(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 on attachment 8552171 [details] [diff] [review]
patch 2 - Fix 64bit shift warning in imgFrame

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

Comment 7

4 years ago
(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

4 years ago
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?
(Assignee)

Comment 9

4 years ago
(In reply to neil@parkwaycc.co.uk 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.

Updated

3 years ago
Attachment #8552172 - Flags: review?(jwalden+bmo) → review+
Attachment #8552171 - Flags: review?(tnikkel) → review+
Attachment #8552170 - Flags: review?(dbolter) → review+

Comment 13

3 years ago
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.
Attachment #8552173 - Flags: review?(benjamin) → review?(nfroyd)
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.
Attachment #8552173 - Flags: review?(nfroyd) → review+
Blocks: 1126017
(Assignee)

Updated

3 years ago
Keywords: leave-open
(Assignee)

Comment 17

3 years ago
Created attachment 8555480 [details] [diff] [review]
patch 5 - Fix 64bit shift warning in nsVoidArray

Fix the same warning as comment 7.
(Assignee)

Updated

3 years ago
Attachment #8555480 - Flags: review?(nfroyd)
Attachment #8555480 - Flags: review?(nfroyd) → review+
__unaligned is meaningless in the amd64 build; it's only relevant to the Itanium compiler. Hacking around the warning is fine.
(Assignee)

Updated

3 years ago
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/a53de22d85ad
Assignee: nobody → quanxunzhen
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.