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

Combine no longer available in Visual Studio 2017 #1352

Closed
BigSeb opened this issue Dec 8, 2017 · 5 comments
Closed

Combine no longer available in Visual Studio 2017 #1352

BigSeb opened this issue Dec 8, 2017 · 5 comments

Comments

@BigSeb
Copy link

BigSeb commented Dec 8, 2017

By removing the C4996 mentionned in the pull request #1348 which solved the tr1::tuple deprecated warning, Combine is no longer available. This is due to the following line in gtest-port.h:

// Determines whether to support Combine(). This only makes sense when
// value-parameterized tests are enabled.  The implementation doesn't
// work on Sun Studio since it doesn't understand templated conversion
// operators.
#if GTEST_HAS_PARAM_TEST && GTEST_HAS_TR1_TUPLE && !defined(__SUNPRO_CC)
# define GTEST_HAS_COMBINE 1
#endif

Because GTEST_HAS_TR1_TUPLE is not activated, GTEST_HAS_COMBINE is not set, disabling testing::Combine even though is was working fine in Visual Studio 2017 before.

dneto0 added a commit to dneto0/effcee that referenced this issue Dec 11, 2017
Our tests use ::testing::Combine from googletest.  We only care
to run in the environments where that is available, i.e. all reasonably
new compilers and runtimes.

Work around the accidental disabling of ::testing::Combine in VS 2017.
See google/googletest#1352
dneto0 added a commit to dneto0/SPIRV-Tools that referenced this issue Dec 11, 2017
Work around faulty logic in googletest, where ::testing::Combine
is accidentally disabled for VS 2017.
See google/googletest#1352
dneto0 added a commit to dneto0/SPIRV-Tools that referenced this issue Dec 11, 2017
Work around faulty logic in googletest, where ::testing::Combine
is accidentally disabled for VS 2017.
See google/googletest#1352
@reid-p
Copy link

reid-p commented Dec 12, 2017

gtest-port.h should probably have this:

#if GTEST_HAS_PARAM_TEST && (GTEST_HAS_STD_TUPLE_ || GTEST_HAS_TR1_TUPLE) && !defined(__SUNPRO_CC)
# define GTEST_HAS_COMBINE 1
#endif

antiagainst pushed a commit to KhronosGroup/SPIRV-Tools that referenced this issue Dec 12, 2017
Work around faulty logic in googletest, where ::testing::Combine
is accidentally disabled for VS 2017.
See google/googletest#1352
s-perron pushed a commit to s-perron/SPIRV-Tools that referenced this issue Dec 14, 2017
Work around faulty logic in googletest, where ::testing::Combine
is accidentally disabled for VS 2017.
See google/googletest#1352
@gennadiycivil
Copy link
Contributor

I have been cleaning up older and inactive GitHub googletest issues. You may see an issue "closed" if it appears to be inactive/abandoned
Thank you

@tlemo
Copy link

tlemo commented Dec 27, 2018

This bug is affecting cross-platform code (it seems that people still need to workaround this).

The fix should be simple (as suggested by reid-p@), so why not just fix it?

@sbenzaquen
Copy link
Collaborator

sbenzaquen commented Dec 28, 2018 via email

@tlemo
Copy link

tlemo commented Dec 28, 2018

Nevermind, I was looking at an older snapshot. I can confirm that it's no longer an issue, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants