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

Additional Information Regarding VS2017 C++17 vs WinMerge #48

Closed
GreyMerlin opened this issue Dec 11, 2017 · 4 comments
Closed

Additional Information Regarding VS2017 C++17 vs WinMerge #48

GreyMerlin opened this issue Dec 11, 2017 · 4 comments

Comments

@GreyMerlin
Copy link
Contributor

Thank You for merging my Pull Request #47. I fear that it is just the first crack in a "Pandora's Box" situation.

I have just today discovered this note (https://blogs.msdn.microsoft.com/vcblog/2017/12/08/c17-feature-removals-and-deprecations/ dated 8 December 2017) on MSDN's Visual C++ Team Blog.

It explains in much greater detail how VS2017 (and beyond) will be approaching the C++14 and C++17 standards, especially the features that have been and will be deprecated. Pay particular attention to comments regarding std:tr1, which is the issue addressed in PR #47; note that this is really a GoogleTest framework problem which that project will have to solve.

As mentioned in PR #47, I was not able to eliminate the problem by changing the language setting to C++14. I now see that, beginning with VS2017 version 15.5, the new deprecation warning is forced to be an new error by C++14 (the default language), in preparation for the eventual elimination of the std:tr1 feature in C++17.

  • So today I made a quick test: I explicitly set the "C++ Language Standard" property to C++17 for our UnitTests project. Alas .... there are now 1608 new compile errors. Most, but not all, are in the GoogleTest framework. Many, perhaps half, can be silenced by defining one (or more) of the new _SILENCE_CXX17_xxx symbols. Most of the remaining errors directly relate to the fact that indeed C++17 no longer supports the std::tr1 construct (which the GoogleTest framework uses extensively). But there remains five new errors, all in cregexp_poco.cpp that all relate to the C++17 elimination of std::auto_ptr

  • So I decided to make another quick test. I set the language for the Merge project to C++17. Alas ... there are now 618 new errors! Most can be silenced by defining one or more of the _SILENCE_CXX17_xxx symbols (as with UnitTests); these new silence-able errors are all in the boost library!! There remain 11 errors that cannot be silenced: eight in cregexp_poco.cpp and three in MergeDoc.cpp.

So ... all of this leaves me with a number of thoughts ...

  • WinMerge has no immediate need to have the "C++ Language Standard" property set to C++17. It should be left at the VS2017 15.5 default, or be explicitly set to VS++14. And all will continue to compile correctly until some time in the future when C++17 is forced upon us (or a future C++14 adds more deprecation errors).

  • Now that these "future" C++17 problems have been detected, we should have a plan, even if informal and vague, on how we want to handle this.

    • The problems in MergeDoc.cpp should be straightforward and easy to fix. (I will attempt to fix.)
    • The problems in cregexp_poco.cpp should be straightforward and easy to fix. This would add a local changes to the poco software; I'm not aware of any previous significant local changes. It may be that poco has already solved this particular problem; if so we should adopt their code. (I will look into this and/or attempt to fix it directly.)
    • I doubt that we have any changes to the boost library other than my (_MSC_VER > 1912) kludge (at least I hope nothing of significance). Since boost is an active development project, we should wait until they make the appropriate changes.
    • To the extent that poco and/or boost have addressed these issues, we should study the effort necessary to upgrade to newer versions of these libraries. I am not really familiar with either of these libraries in a deep technical way; I guess that I should study them and broaden my horizons.
    • I haven't yet tried the other solutions, namely FreeImage and WinIMerge and a few others, to see if this deprecation issue exists with C++17. I know that they all compile cleanly with VS2017 15.5.1 using the default C++14 standard.

Thank you all for your attention.

I would appreciate comments and feedback!

@sdottaka
Copy link
Member

Thank you for the information.
I have not been able to update VisualStudio 2017 yet because I am currently losing fast and cheap internet lines. I will look into this problem when I can update it.

Patches to upgrade boost, poco etc are welcome.

@GreyMerlin
Copy link
Contributor Author

My comments regarding the use of C++17 were overly negative.

The 618 "new" errors (while compiling the Merge project) are because C++17 is flagging as deprecated (with warning, elevated to an error) various existing features that will be deleted in some future C++ standard (possibly C++20); they are still valid in C++17. Simply adding _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to the Preprocessor Definitions compilation property acknowledges that the warnings have been noticed, and should be ignored.

The remaining 11 errors that cannot be silenced (eight in cregexp_poco.cpp and three in MergeDoc.cpp) are legitimate errors in C++17. The changes are straightforward (actually trivial); I will be submitting a Pull Request shortly to incorporate these changes. This will force the Merge project to be compiled with C++17 language rules. As discussed above, the UnitTests project cannot be compiled with C++17, so the compilation will be explicitly based on C++14.

None of these changes impact VS2015; the patches to cregexp_poco.cpp and MergeDoc.cpp are completely legitimate in that environment as well.

@GreyMerlin
Copy link
Contributor Author

I have done more study into the GoogleTest framework vs C++17 issue. As mentioned above, there are two general categories of errors ...

  1. The C++17 standard has eliminated the std::tr1 class
  2. There are hundreds of deprecation warnings that the compiler elevates to C4996 errors.

There are simple solutions to both ...

  1. The GoogleTest framework can disable use of the std::tr1 class entirely! This is disabled by specifying GTEST_HAS_TR1_TUPLE=0; in the Preprocessor Definition compilation property. See here. Our WinMerge project does not use TR1 tuples.
  2. The deprecation warnings are to note features scheduled to be deimplemented in some future C++ standard (possibly as early as C++20). They remain valid features in C++17. These warnings (and the resulting errors) can be removed by specifying _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS; in the Preprocessor Definition compilation property. This allows us to watch and study how other projects react to the necessary changes (mostly related to std::function_name::_Unchecked_Iterators::).

I have simple patches that accomplish both of these solutions, allowing the UnitTests project and the Test configuration of the Merge project to both compile using the C++17 language specification. I will be submitting a Pull Request shortly.

I recommend that this issue can now be closed.

@GreyMerlin
Copy link
Contributor Author

GreyMerlin commented Dec 27, 2017 via email

sdottaka added a commit that referenced this issue Jun 21, 2019
Update of Lithuanian translation
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

2 participants