Make MSVC generate appropriate __cplusplus macro definition

[tatyana-krasnukha]

Summary

When the option "/Zc:__cplusplus" is not specified explicitly, macro's value is "199711L", and gtest cannot determine c++11 support correctly.

https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus

[tatyana-krasnukha]

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

[tatyana-krasnukha]

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]

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

[tatyana-krasnukha]

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.

[asmith]

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

[tatyana-krasnukha]

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]

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

[tatyana-krasnukha]

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.

llvm/cmake/modules/AddLLVM.cmake

 endfunction(add_llvm_implicit_projects)
 
 # Generic support for adding a unittest.
 function(add_unittest test_suite test_name)
   if( NOT LLVM_BUILD_TESTS )
     set(EXCLUDE_FROM_ALL ON)
   endif()
 
-  # Our current version of gtest does not properly recognize C++11 support
-  # with MSVC, so it falls back to tr1 / experimental classes.  Since LLVM
-  # itself requires C++11, we can safely force it on unconditionally so that
-  # we don't have to fight with the buggy gtest check.
-  add_definitions(-DGTEST_LANG_CXX11=1)
-  add_definitions(-DGTEST_HAS_TR1_TUPLE=0)
-
   include_directories(${LLVM_MAIN_SRC_DIR}/utils/unittest/googletest/include)
   include_directories(${LLVM_MAIN_SRC_DIR}/utils/unittest/googlemock/include)
   if (NOT LLVM_ENABLE_THREADS)
     list(APPEND LLVM_COMPILE_DEFINITIONS GTEST_HAS_PTHREAD=0)
   endif ()
 
   if (SUPPORTS_VARIADIC_MACROS_FLAG)
     list(APPEND LLVM_COMPILE_FLAGS "-Wno-variadic-macros")

llvm/cmake/modules/HandleLLVMOptions.cmake

add_definitions(
     -D_UNICODE
   )
 
   if (LLVM_ENABLE_WERROR)
     append("/WX" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
   endif (LLVM_ENABLE_WERROR)
 
   append("/Zc:inline" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
   
+  # Some projects use the __cplusplus preprocessor macro to check support for
+  # a particular version of the C++ standard. When this option is not specified
+  # explicitly, macro's value is "199711L" that implies C++98 Standard.
+  # https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/
+  append("/Zc:__cplusplus" CMAKE_CXX_FLAGS)
+
   # Allow users to request PDBs in release mode. CMake offeres the
   # RelWithDebInfo configuration, but it uses different optimization settings
   # (/Ob1 vs /Ob2 or -O2 vs -O3). LLVM provides this flag so that users can get
   # PDBs without changing codegen.
   option(LLVM_ENABLE_PDB OFF)
   if (LLVM_ENABLE_PDB AND uppercase_CMAKE_BUILD_TYPE STREQUAL "RELEASE")
     append("/Zi" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
     # /DEBUG disables linker GC and ICF, but we want those in Release mode.

llvm/utils/unittest/CMakeLists.txt

 # Where gtest's .h files can be found.
 include_directories(
   googletest/include
   googletest
   googlemock/include
   googlemock
   )
 
-# LLVM requires C++11 but gtest doesn't correctly detect the availability
-# of C++11 on MSVC, so we force it on.
-add_definitions(-DGTEST_LANG_CXX11=1)
-add_definitions(-DGTEST_HAS_TR1_TUPLE=0)
-
 if(WIN32)
   add_definitions(-DGTEST_OS_WINDOWS=1)
 endif()
 
 # Google Test requires headers which need _ALL_SOURCE to build on AIX
 if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
   remove_definitions("-D_XOPEN_SOURCE=700")
   add_definitions("-D_ALL_SOURCE")