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

fix for VS2017 Preview deprecation of ::tr1::tuple #1311

Closed
wants to merge 7 commits into from
Closed

fix for VS2017 Preview deprecation of ::tr1::tuple #1311

wants to merge 7 commits into from

Conversation

bryanzim
Copy link
Contributor

change static_cast to ImplicitCast_ for consitency
fixes for building with path names containing spaces

change static_cast to ImplicitCast_ for consitency
fixes for building with path names containing spaces
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@bryanzim
Copy link
Contributor Author

bryanzim commented Oct 27, 2017 via email

@googlebot
Copy link

CLAs look good, thanks!

@bryanzim
Copy link
Contributor Author

Can someone review this pull request?

@bryanzim
Copy link
Contributor Author

+1

@bryanzim
Copy link
Contributor Author

Can someone review this with regards to issue #1318.

@KindDragon
Copy link
Contributor

In my experience better if you split this PR to 3: ImplicitCast, spaces and tuple warning.

@@ -1092,8 +1092,8 @@ TEST(PrintTr1TupleTest, VariousSizes) {
::std::tr1::tuple<bool, char, short, testing::internal::Int32, // NOLINT
testing::internal::Int64, float, double, const char*, void*,
std::string>
t10(false, 'a', static_cast<short>(3), 4, 5, 1.5F, -2.5, str,
ImplicitCast_<void*>(NULL), "10");
t10(false, 'a', 3, 4, 5, 1.5F, -2.5, str, ImplicitCast_<void*>(NULL),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would return "ImpicitCast_" here, as some compilers/lints may warn about narrowing conversion

@bryanzim
Copy link
Contributor Author

Thank you for the astute observation. I have corrected.

@blizzard4591
Copy link

The updated version of Visual Studio 2017 is now actively being deployed, please merge at least the part for the tr1 usage!

This is a showstopper for builds running with /WX.

blizzard4591 added a commit to blizzard4591/openMittsu that referenced this pull request Dec 6, 2017
Fixed Google Test, which is broken with the most recent release of Visual Studio, thanks to google/googletest#1311 not being merged.
Fixed a mistake in FindLibsodium.cmake, where the Windows 32bit search directory was misnamed.
@nbtdev
Copy link

nbtdev commented Dec 7, 2017

VS2017 15.5 will not build GTest (more specifically, GMock). If someone is waiting for it to be split into three before merging, I will do it (as it seems that @bryanzim has disappeared).

Note that the TR1 fixes in this and in #1343 cannot work on their own, they seem to depend on other changes already in master.

@bryanzim
Copy link
Contributor Author

bryanzim commented Dec 7, 2017

I am still here just got tired of speaking into a black hole. This pull requests which has been updated with the latest master builds fine with no warnings or errors using the following CMake command line and the latest Visual Studio 2017.

cmake -G "Visual Studio 15 Win64" -DBUILD_GTEST=ON -DBUILD_GMOCK=ON -Dgtest_build_tests=ON -Dgtest_build_samples=ON -Dgmock_build_tests=ON ..

It used to pass all the unit test, and I think in fact it still does but travis is having problems finding compilers at the moment.

GreyMerlin added a commit to GreyMerlin/winmerge-v2 that referenced this pull request Dec 7, 2017
 * Using the latest (and just released) version 15.5.0 of VS2017
introduces the following error into the UnitTests project ...

"error C4996: 'std::tr1': warning STL4002: The non-Standard std::tr1
namespace and TR1-only machinery are deprecated and will be REMOVED.
You can define _SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING to
acknowledge that you have received this warning. "

* MSDN claims that this is a C++17 language standards issue, but I was
not able to solve this problem by specifying the use of the C++14
standard language (the Properties control for this is likewise a new
feature of version 15.5.0) See...
https://docs.microsoft.com/en-us/cpp/cpp-conformance-improvements-2017

* The only simple, short-term solution that I could come up with was to
simply follow the suggestion of defining the _SILENCE_TR1_xxx token as
given above.

* Even though this problem has been discussed in the GoogleTest github
forum since at least 6 June, the underlying problem remains un-resolved
at this hour ...
	See google/googletest#1111
	See google/googletest#1320
	See google/googletest#1311

* I will assume that Google's resolution of this problem will require
WinMerge to adopt the newest GoogleTest framework (which is likely a
good idea anyway).  I will keep an eye on their progress and resolution
and advise accordingly.  In the meantime, this kludge allows WinMerge's
UnitTests project to compile and execute correctly.
@nbtdev
Copy link

nbtdev commented Dec 8, 2017

@bryanzim do we know if no one is looking at it because it's three changes in a single PR? That was the main point of my comment

@bryanzim
Copy link
Contributor Author

bryanzim commented Dec 8, 2017

At the time I made the changes the implicit_cast and VS2017 changes were both required in order to make VS2017 compile without warnings or errors.

The change for spaces I believe is very simple addition. I would like to see them all merged at once. To my knowledge no one has ever looked at this; thus the black hole reference. I have been working off my own fork since the time of the original pull request.

@bryanzim
Copy link
Contributor Author

bryanzim commented Dec 8, 2017

Still updating with master and resolving conflicts but no official review or assignee 👎 . I see that some pieces of this pull request were taken and put in master with no original acknowledgement to this pull request that has been open for almost two months when the initial problem was indicated.

@bryanzim
Copy link
Contributor Author

bryanzim commented Dec 8, 2017

And again all checks passed same as when initially put forth 2 months ago.

johnmcfarlane added a commit to johnmcfarlane/cnl that referenced this pull request Dec 9, 2017
cmake fixes to work around gtest failure under 2017 preview

- google/googletest#1311
@bryanzim bryanzim closed this Dec 11, 2017
augustoproiete pushed a commit to sandboxorg/winmerge that referenced this pull request Feb 6, 2018
 * Using the latest (and just released) version 15.5.0 of VS2017
introduces the following error into the UnitTests project ...

"error C4996: 'std::tr1': warning STL4002: The non-Standard std::tr1
namespace and TR1-only machinery are deprecated and will be REMOVED.
You can define _SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING to
acknowledge that you have received this warning. "

* MSDN claims that this is a C++17 language standards issue, but I was
not able to solve this problem by specifying the use of the C++14
standard language (the Properties control for this is likewise a new
feature of version 15.5.0) See...
https://docs.microsoft.com/en-us/cpp/cpp-conformance-improvements-2017

* The only simple, short-term solution that I could come up with was to
simply follow the suggestion of defining the _SILENCE_TR1_xxx token as
given above.

* Even though this problem has been discussed in the GoogleTest github
forum since at least 6 June, the underlying problem remains un-resolved
at this hour ...
	See google/googletest#1111
	See google/googletest#1320
	See google/googletest#1311

* I will assume that Google's resolution of this problem will require
WinMerge to adopt the newest GoogleTest framework (which is likely a
good idea anyway).  I will keep an eye on their progress and resolution
and advise accordingly.  In the meantime, this kludge allows WinMerge's
UnitTests project to compile and execute correctly.
@nbtdev nbtdev mentioned this pull request Feb 21, 2018
sdottaka pushed a commit to sdottaka/winmerge-v2-jp that referenced this pull request Nov 4, 2018
 * Using the latest (and just released) version 15.5.0 of VS2017
introduces the following error into the UnitTests project ...

"error C4996: 'std::tr1': warning STL4002: The non-Standard std::tr1
namespace and TR1-only machinery are deprecated and will be REMOVED.
You can define _SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING to
acknowledge that you have received this warning. "

* MSDN claims that this is a C++17 language standards issue, but I was
not able to solve this problem by specifying the use of the C++14
standard language (the Properties control for this is likewise a new
feature of version 15.5.0) See...
https://docs.microsoft.com/en-us/cpp/cpp-conformance-improvements-2017

* The only simple, short-term solution that I could come up with was to
simply follow the suggestion of defining the _SILENCE_TR1_xxx token as
given above.

* Even though this problem has been discussed in the GoogleTest github
forum since at least 6 June, the underlying problem remains un-resolved
at this hour ...
	See google/googletest#1111
	See google/googletest#1320
	See google/googletest#1311

* I will assume that Google's resolution of this problem will require
WinMerge to adopt the newest GoogleTest framework (which is likely a
good idea anyway).  I will keep an eye on their progress and resolution
and advise accordingly.  In the meantime, this kludge allows WinMerge's
UnitTests project to compile and execute correctly.
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

Successfully merging this pull request may close these issues.

6 participants