830801 - Add -DNOMINMAX to the default OS_CXXFLAGS on Windows

Status: RESOLVED FIXED

(Core :: General, enhancement)

Description

[Mats Palmgren (inactive)]

Bug 786533 added a few "OS_CXXFLAGS += -DNOMINMAX" to Makefiles[1]
to suppress Windows min/max macros.  It would probably be better to
add that in a central location as the default, since there are
probably just a few places which needs the macros (likely because
#include <algorithm> fails to compile in some configurations; iirc,
it affected building updater.exe and some crashreporter tool)

[1] https://hg.mozilla.org/integration/mozilla-inbound/rev/b59c92bc4059

Comment 1

[Makoto Kato [:m_kato]]

Attachment: Part 1. Set NOMINMAX define as default

Comment 2

[Makoto Kato [:m_kato]]

Attachment: Part 2. Remove NOMINMAX define from moz.build

Comment 3

[Makoto Kato [:m_kato]]

Attachment: Part 3. Remove NOMINMAX define from cpp source except to external code

Comment 4

[Makoto Kato [:m_kato]]

Comment on attachment 8640910 [details] [diff] [review]
Part 1. Set NOMINMAX define as default

When investigating VS2015, it requires adding more NOMINMAX macro to many moz.build to build Gecko.

So we should add NOMINMAX define as default.

Comment 5

[Makoto Kato [:m_kato]]

Comment on attachment 8640911 [details] [diff] [review]
Part 2. Remove NOMINMAX define from moz.build

Remove NOMINMAX from moz.build since NOMINMAX is defined as default

Comment 6

[Makoto Kato [:m_kato]]

Comment on attachment 8640912 [details] [diff] [review]
Part 3. Remove NOMINMAX define from cpp source except to external code

Remove NOMINMAX define from our cpp code except to 3rd party code

Comment 7

[(Away)]

Comment on attachment 8640910 [details] [diff] [review]
Part 1. Set NOMINMAX define as default

I'll leave this to mshal as a build peer.

Comment 8

[Michael Shal [:mshal]]

Comment on attachment 8640910 [details] [diff] [review]
Part 1. Set NOMINMAX define as default

I think it might be helpful to include a link to http://support.microsoft.com/kb/143208 in a comment so we know why the define is necessary (it was included in some of the moz.build files).

Comment 9

[Michael Shal [:mshal]]

Comment on attachment 8640911 [details] [diff] [review]
Part 2. Remove NOMINMAX define from moz.build

It looks like you missed these files:

./toolkit/crashreporter/breakpad-windows-libxul/staticruntime/moz.build:for var in ('UNICODE', 'UNICODE_', 'BREAKPAD_NO_TERMINATE_THREAD', 'NOMINMAX'):

./ipc/chromium/chromium-config.mozbuild:        'NOMINMAX': True,

The rest looks good!

Comment 10

[Michael Shal [:mshal]]

Comment on attachment 8640912 [details] [diff] [review]
Part 3. Remove NOMINMAX define from cpp source except to external code

Do we still need these defines?

./config/msvc-stl-wrapper.template.h:#define NOMINMAX 1
./config/gcc-stl-wrapper.template.h:#define NOMINMAX 1

Comment 11

[Makoto Kato [:m_kato]]

Attachment: Part 2. Remove NOMINMAX define from moz.build

Update adding 2 files

Comment 12

[Makoto Kato [:m_kato]]

Attachment: Part 3. Remove NOMINMAX define from cpp source except to external code

add config/gcc-stl-wrapper.template.h and msvc-stl-wrapper.template.h

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e94582c6247b
https://hg.mozilla.org/integration/mozilla-inbound/rev/0450e3e2125e
https://hg.mozilla.org/integration/mozilla-inbound/rev/40fb01ba8c92

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e94582c6247b
https://hg.mozilla.org/mozilla-central/rev/0450e3e2125e
https://hg.mozilla.org/mozilla-central/rev/40fb01ba8c92