Add natvis files directly to the PDB instead of to the VS solution

[zturner]

Summary

Adding natvis files to the VS solution means that if you use CMake to generate a VS solution, natvis will just work. However, this is strictly inferior to putting the natvis files directly in the PDB. For starters, it only works if you generate a VS project, which not everyone does, but even then it's link.exe specific. PDB files support embedding natvis files, and this way it works everywhere.

I originally found out about this because I thought my natvis files weren't getting loaded because I wasn't getting visualization of Optional<T>. Turns out that visualizer was just broken, so I went ahead and fixed it, and also added a visualizer for Expected<T> at the same time.

Not really sure who to add for a review here, so + a couple of random people

[zturner]

Remove some debug prints I accidentally left in the cmake.

[smeenai]

clang/CMakeLists.txt

345

I'm wondering if it would be nicer to do a check_linker_flag_exists for /natvis. I'm fine with this though.

348

I don't think it makes too much of a difference right now, but for completeness, it would be nice to set CMAKE_MODULE_LINKER_FLAGS as well.

llvm/utils/LLVMVisualizers/llvm.natvis

198 ↗

(On Diff #153256)

I think it would be better to split the changes to this file out into its own diff (perhaps even two diffs, one for the fix and one for the new visualizer).

[zturner]

Removed the 2 natvis fixes and committed those independently. Also the CMakeLists.txt files that were in the LLVMVisualizers and ClangVisualizers folders are no longer needed, so I deleted those.

[smeenai]

LGTM

[Bigcheese]

lgtm

clang/CMakeLists.txt

if (LLVM_COMPILER_IS_GCC_COMPATIBLE)
   endif ()
 
   check_cxx_compiler_flag("-Werror -Wnested-anon-types" CXX_SUPPORTS_NO_NESTED_ANON_TYPES_FLAG)
   if( CXX_SUPPORTS_NO_NESTED_ANON_TYPES_FLAG )
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-nested-anon-types" )
   endif()
 endif ()
 
+if (LINKER_IS_LLD_LINK OR LINKER_IS_MSVC_LINK)
+  set(CMAKE_EXE_LINKER_FLAGS
+    "${CMAKE_EXE_LINKER_FLAGS} /natvis:${CMAKE_CURRENT_SOURCE_DIR}/utils/ClangVisualizers/clang.natvis")
+  set(CMAKE_SHARED_LINKER_FLAGS
+    "${CMAKE_SHARED_LINKER_FLAGS} /natvis:${CMAKE_CURRENT_SOURCE_DIR}/utils/ClangVisualizers/clang.natvis")
+endif()
+
 # Determine HOST_LINK_VERSION on Darwin.
 set(HOST_LINK_VERSION)
 if (APPLE)
   set(LD_V_OUTPUT)
   execute_process(
     COMMAND sh -c "${CMAKE_LINKER} -v 2>&1 | head -1"
     RESULT_VARIABLE HAD_ERROR
     OUTPUT_VARIABLE LD_V_OUTPUT
foreach(target ${CLANG_BOOTSTRAP_TARGETS})
     if(target MATCHES "^stage[0-9]*")
       add_custom_target(${target} DEPENDS ${NEXT_CLANG_STAGE}-${target})
     endif()
 
     ExternalProject_Add_StepTargets(${NEXT_CLANG_STAGE} ${target})
   endforeach()
 endif()
 
-if (LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION)
-  add_subdirectory(utils/ClangVisualizers)
-endif()
 add_subdirectory(utils/hmaptool)
 
 configure_file(
   ${CLANG_SOURCE_DIR}/include/clang/Config/config.h.cmake
   ${CLANG_BINARY_DIR}/include/clang/Config/config.h)

clang/utils/ClangVisualizers/CMakeLists.txt

-# Do this by hand instead of using add_llvm_utilities(), which
-# tries to create a corresponding executable, which we don't want.
-if (LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION)
-  set(CLANG_VISUALIZERS clang.natvis)
-  add_custom_target(ClangVisualizers SOURCES ${CLANG_VISUALIZERS})
-  set_target_properties(ClangVisualizers PROPERTIES FOLDER "Utils")
-endif()

llvm/CMakeLists.txt

 
 option(LLVM_DYLIB_SYMBOL_VERSIONING OFF)
 
 option(LLVM_OPTIMIZED_TABLEGEN "Force TableGen to be built with optimization" OFF)
 if(CMAKE_CROSSCOMPILING OR (LLVM_OPTIMIZED_TABLEGEN AND (LLVM_ENABLE_ASSERTIONS OR CMAKE_CONFIGURATION_TYPES)))
   set(LLVM_USE_HOST_TOOLS ON)
 endif()
 
-if (MSVC_IDE AND NOT (MSVC_VERSION LESS 1900))
-  option(LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION "Configure project to use Visual Studio native visualizers" TRUE)
-else()
-  set(LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION FALSE CACHE INTERNAL "For Visual Studio 2013, manually copy natvis files to Documents\\Visual Studio 2013\\Visualizers" FORCE)
-endif()
-
 if (LLVM_BUILD_INSTRUMENTED OR LLVM_BUILD_INSTRUMENTED_COVERAGE OR
     LLVM_ENABLE_IR_PGO)
   if(NOT LLVM_PROFILE_MERGE_POOL_SIZE)
     # A pool size of 1-2 is probably sufficient on a SSD. 3-4 should be fine
     # for spining disks. Anything higher may only help on slower mediums.
     set(LLVM_PROFILE_MERGE_POOL_SIZE "4")
   endif()
   if(NOT LLVM_PROFILE_FILE_PATTERN)
if( LLVM_INCLUDE_UTILS )
   add_subdirectory(utils/yaml-bench)
 else()
   if ( LLVM_INCLUDE_TESTS )
     message(FATAL_ERROR "Including tests when not building utils will not work.
     Either set LLVM_INCLUDE_UTILS to On, or set LLVM_INCLDE_TESTS to Off.")
   endif()
 endif()
 
-# Use LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION instead of LLVM_INCLUDE_UTILS because it is not really a util
-if (LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION)
-  add_subdirectory(utils/LLVMVisualizers)
+if (LINKER_IS_LLD_LINK OR LINKER_IS_MSVC_LINK)
+  set(CMAKE_EXE_LINKER_FLAGS
+    "${CMAKE_EXE_LINKER_FLAGS} /natvis:${CMAKE_CURRENT_SOURCE_DIR}/utils/LLVMVisualizers/llvm.natvis")
+  set(CMAKE_SHARED_LINKER_FLAGS
+    "${CMAKE_SHARED_LINKER_FLAGS} /natvis:${CMAKE_CURRENT_SOURCE_DIR}/utils/LLVMVisualizers/llvm.natvis")
 endif()
 
 foreach( binding ${LLVM_BINDINGS_LIST} )
   if( EXISTS "${LLVM_MAIN_SRC_DIR}/bindings/${binding}/CMakeLists.txt" )
     add_subdirectory(bindings/${binding})
   endif()
 endforeach()
 

llvm/cmake/modules/HandleLLVMOptions.cmake

 # This CMake module is responsible for interpreting the user defined LLVM_
 # options and executing the appropriate CMake commands to realize the users'
 # selections.
 
 # This is commonly needed so make sure it's defined before we include anything
 # else.
 string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
 
 include(CheckCompilerVersion)
 include(HandleLLVMStdlib)
 include(CheckCCompilerFlag)
 include(CheckCXXCompilerFlag)
 
-if(CMAKE_LINKER MATCHES "lld-link.exe" OR (WIN32 AND LLVM_USE_LINKER STREQUAL "lld"))
+if (WIN32)
+  if(CMAKE_LINKER MATCHES "lld-link.exe" OR (LLVM_USE_LINKER STREQUAL "lld"))
+    set(LINKER_IS_MSVC_LINK FALSE)
     set(LINKER_IS_LLD_LINK TRUE)
+  elseif(CMAKE_LINKER MATCHES "link.exe")
+    set(LINKER_IS_MSVC_LINK TRUE)
+    set(LINKER_IS_LLD_LINK FALSE)
+  else()
+    set(LINKER_IS_MSVC_LINK FALSE)
+    set(LINKER_IS_LLD_LINK FALSE)
+  endif()
 else()
+  set(LINKER_IS_MSVC_LINK FALSE)
   set(LINKER_IS_LLD_LINK FALSE)
 endif()
 
 set(LLVM_ENABLE_LTO OFF CACHE STRING "Build LLVM with LTO. May be specified as Thin or Full to use a particular kind of LTO")
 string(TOUPPER "${LLVM_ENABLE_LTO}" uppercase_LLVM_ENABLE_LTO)
 
 # Ninja Job Pool support
 # The following only works with the Ninja generator in CMake >= 3.0.

llvm/utils/LLVMVisualizers/CMakeLists.txt

-# Do this by hand instead of using add_llvm_utilities(), which
-# tries to create a corresponding executable, which we don't want.
-if (LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION)
-  set(LLVM_VISUALIZERS llvm.natvis)
-  add_custom_target(LLVMVisualizers SOURCES ${LLVM_VISUALIZERS})
-  set_target_properties(LLVMVisualizers PROPERTIES FOLDER "Utils")
-endif()