[Build bustage] Remove old Windows defines from WinMessages.h

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
a year ago
a year ago

People

(Reporter: fatseng, Assigned: fatseng)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

User Story

The issue here is that our WinMessages.h header will cause build bustage if it is included in the expanded include tree before Windows headers like Winuser.h or windows.h. Specifically, to support old Windows SDKs it checks if certain windows defines are defined and if not it defines them. If a more recent Windows header is then included after WinMessages.h we get a compile time error because the Windows header defines the defines unconditionally.

Having include order matter is bad since build errors can bite people unexpectedly and in ways that can waste non-trivial amounts of their time.

Since we now require VS 2015 to build, I believe all these defines in WinMessages.h are no longer necessary. We should just remove them and eliminate this issue.

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

a year ago
Assignee: nobody → fatseng
(Assignee)

Updated

a year ago
Status: NEW → ASSIGNED
(Assignee)

Updated

a year ago
Blocks: 1345710
Comment hidden (mozreview-request)
(Assignee)

Comment 2

a year ago
Comment on attachment 8853362 [details]
Bug 1352368 - Remove some old Windows defines from WinMessages.h

There are some redefintion warnings as following.
Remove old Windows defines from WinMessages.h to fix them.

1:50.12 Unified_cpp_widget_windows3.cpp
 1:50.12 Warning: C4005 in C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h: 'WM_GETOBJECT': macro redefinition
 1:50.12 C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h(2032): warning C4005: 'WM_GETOBJECT': macro redefinition
 1:50.12 c:\mozilla-source\gecko-0215\obj-i686-pc-mingw32\dist\include\mozilla/widget/WinMessages.h(72): note: see previous definition of 'WM_GETOBJECT'
 1:50.12 Warning: C4005 in C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h: 'MAPVK_VK_TO_VSC': macro redefinition
 1:50.12 C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h(6663): warning C4005: 'MAPVK_VK_TO_VSC': macro redefinition
 1:50.12 c:\mozilla-source\gecko-0215\obj-i686-pc-mingw32\dist\include\mozilla/widget/WinMessages.h(100): note: see previous definition of 'MAPVK_VK_TO_VSC'
 1:50.12 Warning: C4005 in C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h: 'MAPVK_VSC_TO_VK': macro redefinition
 1:50.12 C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h(6664): warning C4005: 'MAPVK_VSC_TO_VK': macro redefinition
 1:50.12 c:\mozilla-source\gecko-0215\obj-i686-pc-mingw32\dist\include\mozilla/widget/WinMessages.h(101): note: see previous definition of 'MAPVK_VSC_TO_VK'
 1:50.12 Warning: C4005 in C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h: 'MAPVK_VK_TO_CHAR': macro redefinition
 1:50.12 C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h(6665): warning C4005: 'MAPVK_VK_TO_CHAR': macro redefinition
 1:50.12 c:\mozilla-source\gecko-0215\obj-i686-pc-mingw32\dist\include\mozilla/widget/WinMessages.h(102): note: see previous definition of 'MAPVK_VK_TO_CHAR'
 1:50.12 Warning: C4005 in C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h: 'MAPVK_VSC_TO_VK_EX': macro redefinition
 1:50.12 C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h(6666): warning C4005: 'MAPVK_VSC_TO_VK_EX': macro redefinition
 1:50.12 c:\mozilla-source\gecko-0215\obj-i686-pc-mingw32\dist\include\mozilla/widget/WinMessages.h(103): note: see previous definition of 'MAPVK_VSC_TO_VK_EX'
 1:50.12 Warning: C4005 in C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h: 'MAPVK_VK_TO_VSC_EX': macro redefinition
 1:50.12 C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h(6669): warning C4005: 'MAPVK_VK_TO_VSC_EX': macro redefinition
 1:50.12 c:\mozilla-source\gecko-0215\obj-i686-pc-mingw32\dist\include\mozilla/widget/WinMessages.h(104): note: see previous definition of 'MAPVK_VK_TO_VSC_EX'
Attachment #8853362 - Flags: review?(jwatt)
User Story: (updated)

Comment 3

a year ago
mozreview-review
Comment on attachment 8853362 [details]
Bug 1352368 - Remove some old Windows defines from WinMessages.h

https://reviewboard.mozilla.org/r/125480/#review128020

::: commit-message-7cb42:1
(Diff revision 1)
> +Bug 1352368 - Remove old Windows defines from WinMessages.h

I think this should be "Remove some old Windows defines from WinMessage.h" since it looks like we're leaving other ones.

::: commit-message-7cb42:2
(Diff revision 1)
> +Bug 1352368 - Remove old Windows defines from WinMessages.h
> +We require Visual Studio 2015 to build, so we no longer need these defines in WinMessages.h

Make this "These defines cause include ordering issues that can result in build errors if WinMessages.h is included before certain Windows headers. To solve this issue this commit simply removes those defines since nowadays we require Visua Studio 2015 to build and no longer need them."

And blank line between the summary line and the extended comment, plesae.
Attachment #8853362 - Flags: review?(jwatt) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 6

a year ago
mozreview-review
Comment on attachment 8853362 [details]
Bug 1352368 - Remove some old Windows defines from WinMessages.h

https://reviewboard.mozilla.org/r/125480/#review128138
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 7

a year ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8c9fcd6aef01
Remove some old Windows defines from WinMessages.h r=jwatt
Keywords: checkin-needed

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8c9fcd6aef01
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.