cld2_dynamic_data.cc and cld2_dynamic_data_loader.cc problems on Win32

[GoogleCodeExporter]

Chromium build output from one of the buildbots:

FAILED: ninja -t msvc -e environment.x86 -- E:\b\build\goma/gomacc 
"E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64_x86\cl.exe" /nologo 
/showIncludes /FC 
@obj\third_party\cld_2\src\internal\cld2_dynamic.cld2_dynamic_data.obj.rsp /c 
..\..\third_party\cld_2\src\internal\cld2_dynamic_data.cc 
/Foobj\third_party\cld_2\src\internal\cld2_dynamic.cld2_dynamic_data.obj 
/Fdobj\third_party\cld_2\cld2_dynamic.cc.pdb 
e:\b\build\slave\win\build\src\third_party\cld_2\src\internal\cld2_dynamic_data.
cc(33) : error C2039: 'max' : is not a member of 'std'
e:\b\build\slave\win\build\src\third_party\cld_2\src\internal\cld2_dynamic_data.
cc(33) : error C3861: 'max': identifier not found
e:\b\build\slave\win\build\src\third_party\cld_2\src\internal\cld2_dynamic_data.
cc(85) : warning C4018: '<' : signed/unsigned mismatch
FAILED: ninja -t msvc -e environment.x86 -- E:\b\build\goma/gomacc 
"E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64_x86\cl.exe" /nologo 
/showIncludes /FC 
@obj\third_party\cld_2\src\internal\cld2_dynamic.cld2_dynamic_data_loader.obj.rs
p /c ..\..\third_party\cld_2\src\internal\cld2_dynamic_data_loader.cc 
/Foobj\third_party\cld_2\src\internal\cld2_dynamic.cld2_dynamic_data_loader.obj 
/Fdobj\third_party\cld_2\cld2_dynamic.cc.pdb 
e:\b\build\slave\win\build\src\third_party\cld_2\src\internal\cld2_dynamic_data_
loader.cc(99) : error C2220: warning treated as error - no 'object' file 
generated
e:\b\build\slave\win\build\src\third_party\cld_2\src\internal\cld2_dynamic_data_
loader.cc(99) : warning C4018: '<' : signed/unsigned mismatch
e:\b\build\slave\win\build\src\third_party\cld_2\src\internal\cld2_dynamic_data_
loader.cc(235) : warning C4018: '<' : signed/unsigned mismatch

This should be fixed.

Original issue reported on code.google.com by andrewha...@chromium.org on 1 Oct 2014 at 2:36

[GoogleCodeExporter]

Taking a look at this since it prevents building the dynamic CLD2 on Windows.

Original comment by andrewha...@google.com on 1 Oct 2014 at 2:36

• Changed state: Started

[GoogleCodeExporter]

This looks like it could be:
http://stackoverflow.com/questions/2789481/problem-calling-stdmax

Basically it looks like windows.h defines "min" and "max" macros, which need to 
be guarded against in this code.

Original comment by andrewha...@google.com on 1 Oct 2014 at 2:38

[GoogleCodeExporter]

Original comment by andrewha...@google.com on 1 Oct 2014 at 2:40

• Changed title: cld2_dynamic_data_loader.cc doesn't compile on win32 in Chromium

[GoogleCodeExporter]

Original comment by andrewha...@google.com on 1 Oct 2014 at 2:40

• Changed title: cld2_dynamic_data.cc and cld2_dynamic_data_loader.cc problems on Win32

[GoogleCodeExporter]

Patch ready; compile_and_test_all.sh reports success for all dynamic and static 
unit tests, so I'm going to go ahead and commit.

Original comment by andrewha...@google.com on 1 Oct 2014 at 2:48

[GoogleCodeExporter]

Original comment by andrewha...@google.com on 1 Oct 2014 at 2:48

Attachments:

• cld2patch.txt

[GoogleCodeExporter]

Actually, the patch I uploaded - though it compiles and runs on Linux - won't 
fix the windows problem. What's current there is the use of std::max without 
the appropriate include on Windows; but the include on Windows will define 
macros that will break with the current syntax.

Alternative solution to avoid sucking in #ifdefs for win32: do it with the 
ternary operator. This is test code, so this should not be a big deal.

Original comment by andrewha...@google.com on 1 Oct 2014 at 3:03

[GoogleCodeExporter]

Original comment by andrewha...@google.com on 1 Oct 2014 at 3:04

Attachments:

• cld2patch_v2.txt

[GoogleCodeExporter]

Committed as r169 and will roll into Chromium to confirm the fix.

Original comment by andrewha...@google.com on 1 Oct 2014 at 3:06

[GoogleCodeExporter]

Confirmed, this is no longer an issue in the Chromium windows bots.

Original comment by andrewha...@google.com on 24 Oct 2014 at 6:57

• Changed state: Fixed