[clang] Revise IDE folder structure #89743
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-clang-analysis Author: Michael Kruse (Meinersbur) ChangesReviewers of #89153 suggested to break up the patch into per-subproject patches. This is the Clang part. See #89153 for the entire series and motivation. Update the folder titles for targets in the monorepository that have not seen taken care of for some time. These are the folders that targets are organized in Visual Studio and XCode (
Full diff: https://github.com/llvm/llvm-project/pull/89743.diff 13 Files Affected:
diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index f092766fa19f07..09da3ad9979ffd 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -1,4 +1,5 @@
cmake_minimum_required(VERSION 3.20.0)
+set(LLVM_SUBPROJECT_TITLE "Clang")
if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
@@ -390,7 +391,7 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
# Installing the headers needs to depend on generating any public
# tablegen'd headers.
add_custom_target(clang-headers DEPENDS clang-tablegen-targets)
- set_target_properties(clang-headers PROPERTIES FOLDER "Misc")
+ set_target_properties(clang-headers PROPERTIES FOLDER "Clang/Resources")
if(NOT LLVM_ENABLE_IDE)
add_llvm_install_targets(install-clang-headers
DEPENDS clang-headers
@@ -398,6 +399,7 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
endif()
add_custom_target(bash-autocomplete DEPENDS utils/bash-autocomplete.sh)
+ set_target_properties(bash-autocomplete PROPERTIES FOLDER "Clang/Misc")
install(FILES utils/bash-autocomplete.sh
DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
COMPONENT bash-autocomplete)
@@ -478,7 +480,7 @@ add_custom_target(clang-tablegen-targets
omp_gen
ClangDriverOptions
${CLANG_TABLEGEN_TARGETS})
-set_target_properties(clang-tablegen-targets PROPERTIES FOLDER "Misc")
+set_target_properties(clang-tablegen-targets PROPERTIES FOLDER "Clang/Tablegenning/Targets")
list(APPEND LLVM_COMMON_DEPENDS clang-tablegen-targets)
# Force target to be built as soon as possible. Clang modules builds depend
@@ -541,7 +543,7 @@ endif()
# Custom target to install all clang libraries.
add_custom_target(clang-libraries)
-set_target_properties(clang-libraries PROPERTIES FOLDER "Misc")
+set_target_properties(clang-libraries PROPERTIES FOLDER "Clang/Install")
if(NOT LLVM_ENABLE_IDE)
add_llvm_install_targets(install-clang-libraries
diff --git a/clang/bindings/python/tests/CMakeLists.txt b/clang/bindings/python/tests/CMakeLists.txt
index c4cd2539e9d6cf..2543cf739463d9 100644
--- a/clang/bindings/python/tests/CMakeLists.txt
+++ b/clang/bindings/python/tests/CMakeLists.txt
@@ -11,7 +11,7 @@ add_custom_target(check-clang-python
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/..)
set(RUN_PYTHON_TESTS TRUE)
-set_target_properties(check-clang-python PROPERTIES FOLDER "Clang tests")
+set_target_properties(check-clang-python PROPERTIES FOLDER "Clang/Tests")
# Tests require libclang.so which is only built with LLVM_ENABLE_PIC=ON
if(NOT LLVM_ENABLE_PIC)
diff --git a/clang/cmake/modules/AddClang.cmake b/clang/cmake/modules/AddClang.cmake
index 75b0080f671564..a5ef639187d9db 100644
--- a/clang/cmake/modules/AddClang.cmake
+++ b/clang/cmake/modules/AddClang.cmake
@@ -26,7 +26,6 @@ function(clang_tablegen)
if(CTG_TARGET)
add_public_tablegen_target(${CTG_TARGET})
- set_target_properties( ${CTG_TARGET} PROPERTIES FOLDER "Clang tablegenning")
set_property(GLOBAL APPEND PROPERTY CLANG_TABLEGEN_TARGETS ${CTG_TARGET})
endif()
endfunction(clang_tablegen)
@@ -138,13 +137,11 @@ macro(add_clang_library name)
endif()
endforeach()
- set_target_properties(${name} PROPERTIES FOLDER "Clang libraries")
set_clang_windows_version_resource_properties(${name})
endmacro(add_clang_library)
macro(add_clang_executable name)
add_llvm_executable( ${name} ${ARGN} )
- set_target_properties(${name} PROPERTIES FOLDER "Clang executables")
set_clang_windows_version_resource_properties(${name})
endmacro(add_clang_executable)
diff --git a/clang/docs/CMakeLists.txt b/clang/docs/CMakeLists.txt
index 4163dd2d90ad5b..51e9db29f887f3 100644
--- a/clang/docs/CMakeLists.txt
+++ b/clang/docs/CMakeLists.txt
@@ -78,6 +78,7 @@ if (LLVM_ENABLE_DOXYGEN)
COMMAND ${DOXYGEN_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/doxygen.cfg
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
COMMENT "Generating clang doxygen documentation." VERBATIM)
+ set_target_properties(doxygen-clang PROPERTIES FOLDER "Clang/Docs")
if (LLVM_BUILD_DOCS)
add_dependencies(doxygen doxygen-clang)
diff --git a/clang/lib/Analysis/FlowSensitive/CMakeLists.txt b/clang/lib/Analysis/FlowSensitive/CMakeLists.txt
index 6631fe27f3d901..4e9262c4e6f186 100644
--- a/clang/lib/Analysis/FlowSensitive/CMakeLists.txt
+++ b/clang/lib/Analysis/FlowSensitive/CMakeLists.txt
@@ -36,3 +36,4 @@ add_custom_command(OUTPUT HTMLLogger.inc
DEPENDS ${CLANG_SOURCE_DIR}/utils/bundle_resources.py HTMLLogger.html HTMLLogger.css HTMLLogger.js
VERBATIM)
add_custom_target(clangAnalysisFlowSensitiveResources DEPENDS HTMLLogger.inc)
+set_target_properties(clangAnalysisFlowSensitiveResources PROPERTIES FOLDER "Clang/Misc")
diff --git a/clang/lib/Headers/CMakeLists.txt b/clang/lib/Headers/CMakeLists.txt
index e6ae4e19e81db9..d7cdaf39c1159e 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -439,14 +439,14 @@ endforeach( f )
function(add_header_target target_name file_list)
add_library(${target_name} INTERFACE ${file_list})
set_target_properties(${target_name} PROPERTIES
- FOLDER "Misc"
+ FOLDER "Clang/Resources"
RUNTIME_OUTPUT_DIRECTORY "${output_dir}")
endfunction()
# The catch-all clang-resource-headers target
add_library(clang-resource-headers INTERFACE ${out_files})
set_target_properties("clang-resource-headers" PROPERTIES
- FOLDER "Misc"
+ FOLDER "Clang/Resources"
RUNTIME_OUTPUT_DIRECTORY "${output_dir}")
add_dependencies("clang-resource-headers"
"core-resource-headers"
diff --git a/clang/lib/Tooling/CMakeLists.txt b/clang/lib/Tooling/CMakeLists.txt
index 91e6cbdcbc44f7..1fa3498a862c12 100644
--- a/clang/lib/Tooling/CMakeLists.txt
+++ b/clang/lib/Tooling/CMakeLists.txt
@@ -75,6 +75,7 @@ else()
add_custom_target(run-ast-api-dump-tool
DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/ASTNodeAPI.json
)
+ set_target_properties(run-ast-api-dump-tool PROPERTIES FOLDER "Clang/Tablegenning")
add_custom_command(
COMMENT Generate NodeIntrospection.inc
@@ -99,6 +100,7 @@ else()
DEPENDS
${BINARY_INCLUDE_DIR}/NodeIntrospection.inc
)
+ set_target_properties(run-ast-api-generate-tool PROPERTIES FOLDER "Clang/Tablegenning")
endif()
add_clang_library(clangTooling
diff --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index df34a5707da33e..5fceb1d7103341 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -170,7 +170,7 @@ configure_file(AST/gen_ast_dump_json_test.py
${CLANG_BINARY_DIR}/bin/gen_ast_dump_json_test.py COPYONLY)
add_custom_target(clang-test-depends DEPENDS ${CLANG_TEST_DEPS})
-set_target_properties(clang-test-depends PROPERTIES FOLDER "Clang tests")
+set_target_properties(clang-test-depends PROPERTIES FOLDER "Clang/Tests")
add_lit_testsuite(check-clang "Running the Clang regression tests"
${CMAKE_CURRENT_BINARY_DIR}
@@ -179,7 +179,6 @@ add_lit_testsuite(check-clang "Running the Clang regression tests"
DEPENDS ${CLANG_TEST_DEPS}
ARGS ${CLANG_TEST_EXTRA_ARGS}
)
-set_target_properties(check-clang PROPERTIES FOLDER "Clang tests")
add_lit_testsuites(CLANG ${CMAKE_CURRENT_SOURCE_DIR}
PARAMS ${CLANG_TEST_PARAMS}
@@ -190,7 +189,7 @@ add_lit_testsuites(CLANG ${CMAKE_CURRENT_SOURCE_DIR}
# Add a legacy target spelling: clang-test
add_custom_target(clang-test)
add_dependencies(clang-test check-clang)
-set_target_properties(clang-test PROPERTIES FOLDER "Clang tests")
+set_target_properties(clang-test PROPERTIES FOLDER "Clang/Tests")
# FIXME: This logic can be removed once all buildbots have moved
# debuginfo-test from clang/test to llvm/projects or monorepo.
diff --git a/clang/tools/libclang/CMakeLists.txt b/clang/tools/libclang/CMakeLists.txt
index b5b6d2807d714c..7b634003d11f42 100644
--- a/clang/tools/libclang/CMakeLists.txt
+++ b/clang/tools/libclang/CMakeLists.txt
@@ -230,7 +230,7 @@ install(DIRECTORY ../../include/clang-c
# component and an install-component target, so add a dummy libclang-headers
# target to allow using it in LLVM_DISTRIBUTION_COMPONENTS.
add_custom_target(libclang-headers)
-set_target_properties(libclang-headers PROPERTIES FOLDER "Misc")
+set_target_properties(libclang-headers PROPERTIES FOLDER "Clang/Resources")
if (NOT LLVM_ENABLE_IDE)
add_llvm_install_targets(install-libclang-headers
diff --git a/clang/unittests/CMakeLists.txt b/clang/unittests/CMakeLists.txt
index 37ca3107b54774..b907822deb0c24 100644
--- a/clang/unittests/CMakeLists.txt
+++ b/clang/unittests/CMakeLists.txt
@@ -1,5 +1,5 @@
add_custom_target(ClangUnitTests)
-set_target_properties(ClangUnitTests PROPERTIES FOLDER "Clang tests")
+set_target_properties(ClangUnitTests PROPERTIES FOLDER "Clang/Tests")
if(CLANG_BUILT_STANDALONE)
# LLVMTesting* libraries are needed for some of the unittests.
diff --git a/clang/utils/ClangVisualizers/CMakeLists.txt b/clang/utils/ClangVisualizers/CMakeLists.txt
index 16d118a421bad8..c047419050d104 100644
--- a/clang/utils/ClangVisualizers/CMakeLists.txt
+++ b/clang/utils/ClangVisualizers/CMakeLists.txt
@@ -3,5 +3,5 @@
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")
+ set_target_properties(ClangVisualizers PROPERTIES FOLDER "Clang/Misc")
endif()
diff --git a/clang/utils/TableGen/CMakeLists.txt b/clang/utils/TableGen/CMakeLists.txt
index 2ca4a96cadb670..5b072a1ac19698 100644
--- a/clang/utils/TableGen/CMakeLists.txt
+++ b/clang/utils/TableGen/CMakeLists.txt
@@ -27,5 +27,3 @@ add_tablegen(clang-tblgen CLANG
)
target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
-
-set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
diff --git a/clang/utils/hmaptool/CMakeLists.txt b/clang/utils/hmaptool/CMakeLists.txt
index 511268069bd1cf..bb40ca3ea90a92 100644
--- a/clang/utils/hmaptool/CMakeLists.txt
+++ b/clang/utils/hmaptool/CMakeLists.txt
@@ -1,6 +1,6 @@
install(PROGRAMS hmaptool DESTINATION "${CLANG_TOOLS_INSTALL_DIR}" COMPONENT hmaptool)
add_custom_target(hmaptool ALL DEPENDS "hmaptool")
-set_target_properties(hmaptool PROPERTIES FOLDER "Utils")
+set_target_properties(hmaptool PROPERTIES FOLDER "Clang/Resources")
if(NOT LLVM_ENABLE_IDE)
add_llvm_install_targets("install-hmaptool"
|
Meinersbur
requested review from
JDevlieghere,
whisperity,
felipepiovezan and
jh7370
There was a problem hiding this comment.
With this patch, I get errors when loading a visual studio solution generated with these change, and all of clang's libraries are placed at the top level. The error is a dialog box saying "The solution already contains an item named 'clang'."
Another thing (that is existing behavior) worth noting is that at some point, we lost the ability to browse directly to files in Visual Studio. Instead of going to Clang Libraries->clangDriver->Source Files->ToolChain.cpp, you have to go to Object Libraries->obj.clangDriver->Source Files->ToolChain.cpp which is pretty strange. Your patch doesn't change this, but if we're correcting folder structure behavior, we should probably address that at the same time if we can.
|
@AaronBallman Thanks for having a look.
I got similar errors with "Unittest" and "UnitTest". I only solved it by using "Unit". I am assuming this is because there is something else already with that name but inconsistent case. I never seen this for this for "Clang"/"clang", but it could be when not all patches from the series are applied. Specifically I think clang-tools-extra may use "Clang"/"clang".
This is how CMake handles object libraries. There is one target There is already an exception for XCode, and we could do the same for MSVC (or add the source files as non-compiled sources to make them show up like |
Ah interesting, thank you for the explanation! I know at one point in time, this used to generate a more obvious layout for the solution file, so I was surprised by the change.
SGTM! It might be useful to explore doing that, but I don't know how many folks are still generating solution files these days. Personally, I use MSVC's integrated CMake support which has its own set of problems but tends to be better than using a solution file (esp in terms of compilation times). |
|
@AaronBallman Would you mind reviewing D89741 ? I was convinced a stakeholder there but they decided to drop out instead of approving it. |
Oh the fun of PR numbers now lining up with Phabricator numbers. I thought you meant https://reviews.llvm.org/D89741 and was very confused until I saw #89741. :-D I'll take a look! |
Well, seems I still have muscle memory from Phabricator times :-) Thanks for the review! |
Meinersbur
requested review from
rafaelauler,
ayermolo,
dcci,
Moxinilian,
ftynse,
makslevental,
stellaraccident,
grypp,
antiagainst,
kuhar,
nicolasvasilache,
hanhanW,
banach-space,
dcaballe,
MaheshRavishankar,
tbaederr,
Endilll,
nikic and
a team
as code owners
Meinersbur
changed the base branch from
users/meinersbur/ide_folders_llvm
to
main
|
Sorry for the notification noise. For some reason, when committing the parent PR, it closed this one. |
As discussed in #89743, when using the Visual Studio solution generators, object library projects are displayed as a collection of non-editable *.obj files. To look for the corresponding source files, one has to browse (or search) to the library's obj.libname project. This patch tries to avoid this as much as possible. For Clang, there is already an exception for XCode. We handle MSVC_IDE the same way. For MLIR, this is more complicated. There are explicit references to the obj.libname target that only work when there is an object library. This patch cleans up the reasons for why an object library is needed: 1. The obj.libname is modified in the calling CMakeLists.txt. Note that with use-only references, `add_library(<name> ALIAS <target>)` could have been used. 2. An `libMLIR.so` (mlir-shlib) is also created. This works by adding linking the object libraries' object files into the libMLIR.so (in addition to the library's own .so/.a). XCode is handled using the `-force_load` linker option instead. Windows is not supported. This mechanism is different from LLVM's llvm-shlib that is created by linking static libraries with `-Wl,--whole-archive` (and `-Wl,-all_load` on MacOS). 3. The library might be added to an aggregate library. In-tree, the seems to be only `libMLIR-C.so` and the standalone example. In XCode, it uses the object library and `-force_load` mechanism as above. Again, this is different from `libLLVM-C.so`. 4. Build an object library whenever it was before this patch, except when generating a Visual Studio solution. This condition could be removed, but I am trying to avoid build breakages of whatever configurations others use. This seems to never have worked with XCode because of the explicit references to obj.libname (reason 1.). I don't have access to XCode, but I tried to preserve the current working. IMHO there should be a common mechanism to build aggregate libraries for all LLVM projects instead of the 4 that we have now. As far as I can see, this means for LLVM there are the following changes on whether object libraries are created: 1. An object library is created even in XCode if FORCE_OBJECT_LIBRARY is set. I do not know how XCode handles it, but I also know CMake will abort otherwise. 2. An object library is created even for explicitly SHARED libraries for building `libMLIR.so`. Again, mlir-shlib does not work otherwise. `libMLIR.so` itself is created using SHARED so this patch is marking it as EXCLUDE_FROM_LIBMLIR. 3. For the second condition, it is now sensitive to whether the mlir-shlib is built at all (LLVM_BUILD_LLVM_DYLIB). However, an object library is still built using the fourth condition unless using the MSVC solution generator. That is, except with MSVC_IDE, when an object library was built before, it will also be an object library now.
As discussed in llvm#89743, when using the Visual Studio solution generators, object library projects are displayed as a collection of non-editable *.obj files. To look for the corresponding source files, one has to browse (or search) to the library's obj.libname project. This patch tries to avoid this as much as possible. For Clang, there is already an exception for XCode. We handle MSVC_IDE the same way. For MLIR, this is more complicated. There are explicit references to the obj.libname target that only work when there is an object library. This patch cleans up the reasons for why an object library is needed: 1. The obj.libname is modified in the calling CMakeLists.txt. Note that with use-only references, `add_library(<name> ALIAS <target>)` could have been used. 2. An `libMLIR.so` (mlir-shlib) is also created. This works by adding linking the object libraries' object files into the libMLIR.so (in addition to the library's own .so/.a). XCode is handled using the `-force_load` linker option instead. Windows is not supported. This mechanism is different from LLVM's llvm-shlib that is created by linking static libraries with `-Wl,--whole-archive` (and `-Wl,-all_load` on MacOS). 3. The library might be added to an aggregate library. In-tree, the seems to be only `libMLIR-C.so` and the standalone example. In XCode, it uses the object library and `-force_load` mechanism as above. Again, this is different from `libLLVM-C.so`. 4. Build an object library whenever it was before this patch, except when generating a Visual Studio solution. This condition could be removed, but I am trying to avoid build breakages of whatever configurations others use. This seems to never have worked with XCode because of the explicit references to obj.libname (reason 1.). I don't have access to XCode, but I tried to preserve the current working. IMHO there should be a common mechanism to build aggregate libraries for all LLVM projects instead of the 4 that we have now. As far as I can see, this means for LLVM there are the following changes on whether object libraries are created: 1. An object library is created even in XCode if FORCE_OBJECT_LIBRARY is set. I do not know how XCode handles it, but I also know CMake will abort otherwise. 2. An object library is created even for explicitly SHARED libraries for building `libMLIR.so`. Again, mlir-shlib does not work otherwise. `libMLIR.so` itself is created using SHARED so this patch is marking it as EXCLUDE_FROM_LIBMLIR. 3. For the second condition, it is now sensitive to whether the mlir-shlib is built at all (LLVM_BUILD_LLVM_DYLIB). However, an object library is still built using the fourth condition unless using the MSVC solution generator. That is, except with MSVC_IDE, when an object library was built before, it will also be an object library now.
Reviewers of #89153 suggested to break up the patch into per-subproject patches. This is the Clang part. See #89153 for the entire series and motivation.
Update the folder titles for targets in the monorepository that have not seen taken care of for some time. These are the folders that targets are organized in Visual Studio and XCode (
set_property(TARGET <target> PROPERTY FOLDER "<title>")) when using the respective CMake's IDE generator.set_property/set_target_property, but are still necessary whenadd_custom_target,add_executable,add_library, etc. are used. A LLVM_SUBPROJECT_TITLE definition is used for that in each subproject's root CMakeLists.txt.