This is an archive of the discontinued LLVM Phabricator instance.

Make MSVC generate appropriate __cplusplus macro definition
ClosedPublic

Authored by tatyana-krasnukha on Jul 17 2020, 6:36 AM.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJul 17 2020, 6:36 AM

No updates, just to trigger rebasing onto master where tests should be already fixed.

It also fixes warnings about using deprecated std::tr1, which became errors in VS 2019, produced while compiling lldbUtilityHelpers project (http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/17887/steps/test/logs/stdio).

asmith accepted this revision.Jul 31 2020, 2:18 PM

I think this should be okay. VS2017 or higher are required to build LLVM and this support is in VS2017.

This revision is now accepted and ready to land.Jul 31 2020, 2:18 PM
tatyana-krasnukha added a comment.EditedAug 3 2020, 4:40 AM

LLVM documentation says: "You will need Visual Studio 2017 or higher, with the latest Update installed". Does this mean Visual Studio 2017 version 15.9? Because the /Zc:__cplusplus option is available starting in Visual Studio 2017 version 15.7.

That's why this patch led to multiple warnings on the lldb-x64-windows-ninja buildbot, which uses Visual Studio 2017 version 15.0, and I don't know whether I should revert the commit or not.

The warning is due to the version of Gtest that LLVM is using.

Here's a discussion of the problem and a solution is to update to Gtest 1.8.1
https://github.com/microsoft/TestAdapterForGoogleTest/issues/119

LLVM is using a fork of Gtest 1.8.0.
https://github.com/llvm/llvm-project/tree/master/llvm/utils/unittest/googletest

The option "/Zc:__cplusplus" solves this problem. With this option passed, Gtest doesn't try using namespace tr1. I don't have such warnings if pass this option.

Though, Gtest 1.8.1 will be required anyway when LLVM will switch to C++17.

asmith added a comment.Aug 3 2020, 3:21 PM

The buildbot is using MSVC 19.16.27042.0 which is Visual Studio 2017 15.9.25.

I see. The option "/Zc:__cplusplus" affects GTEST_LANG_CXX11 but not GTEST_HAS_TR1_TUPLE. I didn't have warnings just because my LLVM fork uses C++17.