Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cld2_dynamic_data.cc and cld2_dynamic_data_loader.cc problems on Win32 #24

Closed
ghost opened this issue Jul 27, 2015 · 10 comments
Closed

cld2_dynamic_data.cc and cld2_dynamic_data_loader.cc problems on Win32 #24

ghost opened this issue Jul 27, 2015 · 10 comments
Labels
Priority-Medium Type-Defect

Comments

@ghost
Copy link

ghost commented Jul 27, 2015

Originally reported on Google Code with ID 24

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.rsp
/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.

Reported by andrewhayden@chromium.org on 2014-10-01 14:36:06
@ghost ghost self-assigned this Jul 27, 2015
@ghost
Copy link
Author

ghost commented Jul 27, 2015 

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

Reported by andrewhayden@google.com on 2014-10-01 14:36:57

  • Status changed: Started

@ghost
Copy link
Author

ghost commented Jul 27, 2015 

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.

Reported by andrewhayden@google.com on 2014-10-01 14:38:43

@ghost
Copy link
Author

ghost commented Jul 27, 2015

Reported by andrewhayden@google.com on 2014-10-01 14:40:01

@ghost
Copy link
Author

ghost commented Jul 27, 2015

Reported by andrewhayden@google.com on 2014-10-01 14:40:55

@ghost
Copy link
Author

ghost commented Jul 27, 2015 

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.

Reported by andrewhayden@google.com on 2014-10-01 14:48:16

@ghost
Copy link
Author

ghost commented Jul 27, 2015

Reported by andrewhayden@google.com on 2014-10-01 14:48:33

- _Attachment: [cld2patch.txt](https://storage.googleapis.com/google-code-attachments/cld2/issue-24/comment-6/cld2patch.txt)_

@ghost
Copy link
Author

ghost commented Jul 27, 2015 

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.

Reported by andrewhayden@google.com on 2014-10-01 15:03:58

@ghost
Copy link
Author

ghost commented Jul 27, 2015

Reported by andrewhayden@google.com on 2014-10-01 15:04:46

- _Attachment: [cld2patch_v2.txt](https://storage.googleapis.com/google-code-attachments/cld2/issue-24/comment-8/cld2patch_v2.txt)_

@ghost
Copy link
Author

ghost commented Jul 27, 2015 

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

Reported by andrewhayden@google.com on 2014-10-01 15:06:35

@ghost
Copy link
Author

ghost commented Jul 27, 2015 

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

Reported by andrewhayden@google.com on 2014-10-24 06:57:34

  • Status changed: Fixed

@ghost ghost closed this as completed Jul 27, 2015
@xiaoluqiaoba xiaoluqiaoba mentioned this issue Aug 1, 2019
Open
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Priority-Medium Type-Defect
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
0 participants