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

[clang-tools-extra] Revise IDE folder structure #89744

Merged
merged 13 commits into from
May 25, 2024

Conversation

Meinersbur
Copy link
Member

Reviewers of #89153 suggested to break up the patch into per-subproject patches. This is the Clang-tools-extra 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.

  • Ensure that every target is in a folder
  • Use a folder hierarchy with each LLVM subproject as a top-level folder
  • Use consistent folder names between subprojects
  • When using target-creating functions from AddLLVM.cmake, automatically deduce the folder. This reduces the number of set_property/set_target_property, but are still necessary when add_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.

@Meinersbur Meinersbur mentioned this pull request Apr 23, 2024
@llvmbot
Copy link

llvmbot commented May 21, 2024

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Michael Kruse (Meinersbur)

Changes

Reviewers of #89153 suggested to break up the patch into per-subproject patches. This is the Clang-tools-extra 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 &lt;target&gt; PROPERTY FOLDER "&lt;title&gt;")) when using the respective CMake's IDE generator.

  • Ensure that every target is in a folder
  • Use a folder hierarchy with each LLVM subproject as a top-level folder
  • Use consistent folder names between subprojects
  • When using target-creating functions from AddLLVM.cmake, automatically deduce the folder. This reduces the number of set_property/set_target_property, but are still necessary when add_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.

Full diff: https://github.com/llvm/llvm-project/pull/89744.diff

11 Files Affected:

  • (modified) clang-tools-extra/CMakeLists.txt (+2)
  • (modified) clang-tools-extra/clang-tidy/CMakeLists.txt (+1-1)
  • (modified) clang-tools-extra/clang-tidy/misc/CMakeLists.txt (+2)
  • (modified) clang-tools-extra/clangd/unittests/CMakeLists.txt (+1)
  • (modified) clang-tools-extra/docs/CMakeLists.txt (+1)
  • (modified) clang-tools-extra/include-cleaner/unittests/CMakeLists.txt (+1)
  • (modified) clang-tools-extra/pseudo/include/CMakeLists.txt (+1)
  • (modified) clang-tools-extra/pseudo/tool/CMakeLists.txt (+1)
  • (modified) clang-tools-extra/pseudo/unittests/CMakeLists.txt (+1)
  • (modified) clang-tools-extra/test/CMakeLists.txt (-1)
  • (modified) clang-tools-extra/unittests/CMakeLists.txt (+1-1)
diff --git a/clang-tools-extra/CMakeLists.txt b/clang-tools-extra/CMakeLists.txt
index 6a3f741721ee6..f6a6b57b5ef0b 100644
--- a/clang-tools-extra/CMakeLists.txt
+++ b/clang-tools-extra/CMakeLists.txt
@@ -1,3 +1,5 @@
+set(LLVM_SUBPROJECT_TITLE "Clang Tools Extra")
+
 include(CMakeDependentOption)
 include(GNUInstallDirs)
 
diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt
index 7e1905aa897b7..430ea4cdbb38e 100644
--- a/clang-tools-extra/clang-tidy/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -121,7 +121,7 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
     PATTERN "*.h"
     )
   add_custom_target(clang-tidy-headers)
-  set_target_properties(clang-tidy-headers PROPERTIES FOLDER "Misc")
+  set_target_properties(clang-tidy-headers PROPERTIES FOLDER "Clang Tools Extra/Resources")
   if(NOT LLVM_ENABLE_IDE)
     add_llvm_install_targets(install-clang-tidy-headers
                              DEPENDS clang-tidy-headers
diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index d9ec268650c05..4eda705c45d2a 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -15,6 +15,7 @@ add_custom_command(
     DEPENDS ${clang_tidy_confusable_chars_gen_target} ConfusableTable/confusables.txt)
 
 add_custom_target(genconfusable DEPENDS Confusables.inc)
+set_target_properties(genconfusable PROPERTIES FOLDER "Clang Tools Extra/Tablegenning")
 
 add_clang_library(clangTidyMiscModule
   ConstCorrectnessCheck.cpp
@@ -51,6 +52,7 @@ add_clang_library(clangTidyMiscModule
   genconfusable
   ClangDriverOptions
   )
+set_target_properties(clangTidyMiscModule PROPERTIES FOLDER "Clang Tools Extra/Libraries")
 
 clang_target_link_libraries(clangTidyMiscModule
   PRIVATE
diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt
index 7f1ae5c43d80c..0d4628ccf25d8 100644
--- a/clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -29,6 +29,7 @@ include(${CMAKE_CURRENT_SOURCE_DIR}/../quality/CompletionModel.cmake)
 gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/decision_forest_model DecisionForestRuntimeTest ::ns1::ns2::test::Example)
 
 add_custom_target(ClangdUnitTests)
+set_target_properties(ClangdUnitTests PROPERTIES FOLDER "Clang Tools Extra/Tests")
 add_unittest(ClangdUnitTests ClangdTests
   Annotations.cpp
   ASTTests.cpp
diff --git a/clang-tools-extra/docs/CMakeLists.txt b/clang-tools-extra/docs/CMakeLists.txt
index 8f442e1f661ed..272db266b5054 100644
--- a/clang-tools-extra/docs/CMakeLists.txt
+++ b/clang-tools-extra/docs/CMakeLists.txt
@@ -77,6 +77,7 @@ if (DOXYGEN_FOUND)
       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-tools PROPERTIES FOLDER "Clang Tools Extra/Docs")
 
     if (LLVM_BUILD_DOCS)
       add_dependencies(doxygen doxygen-clang-tools)
diff --git a/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt b/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
index 1e89534b51116..416535649f622 100644
--- a/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
+++ b/clang-tools-extra/include-cleaner/unittests/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_custom_target(ClangIncludeCleanerUnitTests)
+set_target_properties(ClangIncludeCleanerUnitTests PROPERTIES FOLDER "Clang Tools Extra/Tests")
 add_unittest(ClangIncludeCleanerUnitTests ClangIncludeCleanerTests
   AnalysisTest.cpp
   FindHeadersTest.cpp
diff --git a/clang-tools-extra/pseudo/include/CMakeLists.txt b/clang-tools-extra/pseudo/include/CMakeLists.txt
index 2334cfa12e337..1d88af2eb6873 100644
--- a/clang-tools-extra/pseudo/include/CMakeLists.txt
+++ b/clang-tools-extra/pseudo/include/CMakeLists.txt
@@ -29,3 +29,4 @@ add_custom_command(OUTPUT ${cxx_bnf_inc}
 add_custom_target(cxx_gen
     DEPENDS ${cxx_symbols_inc} ${cxx_bnf_inc}
     VERBATIM)
+set_target_properties(cxx_gen PROPERTIES FOLDER "Clang Tools Extra/Tablegenning")
diff --git a/clang-tools-extra/pseudo/tool/CMakeLists.txt b/clang-tools-extra/pseudo/tool/CMakeLists.txt
index 49e1dc29a5a4e..bead383228396 100644
--- a/clang-tools-extra/pseudo/tool/CMakeLists.txt
+++ b/clang-tools-extra/pseudo/tool/CMakeLists.txt
@@ -26,4 +26,5 @@ add_custom_command(OUTPUT HTMLForestResources.inc
   DEPENDS ${CLANG_SOURCE_DIR}/utils/bundle_resources.py HTMLForest.css HTMLForest.js HTMLForest.html
   VERBATIM)
 add_custom_target(clang-pseudo-resources DEPENDS HTMLForestResources.inc)
+set_target_properties(clang-pseudo-resources PROPERTIES FOLDER "Clang Tools Extra/Resources")
 add_dependencies(clang-pseudo clang-pseudo-resources)
diff --git a/clang-tools-extra/pseudo/unittests/CMakeLists.txt b/clang-tools-extra/pseudo/unittests/CMakeLists.txt
index 821ca4d0652e1..53583ceb61864 100644
--- a/clang-tools-extra/pseudo/unittests/CMakeLists.txt
+++ b/clang-tools-extra/pseudo/unittests/CMakeLists.txt
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_custom_target(ClangPseudoUnitTests)
+set_target_properties(ClangPseudoUnitTests PROPERTIES FOLDER "Clang Tools Extra/Tests")
 add_unittest(ClangPseudoUnitTests ClangPseudoTests
   BracketTest.cpp
   CXXTest.cpp
diff --git a/clang-tools-extra/test/CMakeLists.txt b/clang-tools-extra/test/CMakeLists.txt
index f4c529ee8af20..d488c673d1868 100644
--- a/clang-tools-extra/test/CMakeLists.txt
+++ b/clang-tools-extra/test/CMakeLists.txt
@@ -100,7 +100,6 @@ add_lit_testsuite(check-clang-extra "Running clang-tools-extra/test"
    ${CMAKE_CURRENT_BINARY_DIR}
    DEPENDS ${CLANG_TOOLS_TEST_DEPS}
    )
-set_target_properties(check-clang-extra PROPERTIES FOLDER "Clang extra tools' tests")
 
 add_lit_testsuites(CLANG-EXTRA ${CMAKE_CURRENT_SOURCE_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
diff --git a/clang-tools-extra/unittests/CMakeLists.txt b/clang-tools-extra/unittests/CMakeLists.txt
index 086a68e638307..77311540e719f 100644
--- a/clang-tools-extra/unittests/CMakeLists.txt
+++ b/clang-tools-extra/unittests/CMakeLists.txt
@@ -1,5 +1,5 @@
 add_custom_target(ExtraToolsUnitTests)
-set_target_properties(ExtraToolsUnitTests PROPERTIES FOLDER "Extra Tools Unit Tests")
+set_target_properties(ExtraToolsUnitTests PROPERTIES FOLDER "Clang Tools Extra/Tests")
 
 function(add_extra_unittest test_dirname)
   add_unittest(ExtraToolsUnitTests ${test_dirname} ${ARGN})

Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me this entire change doesn't make sense.

@Meinersbur Meinersbur deleted the branch llvm:main May 25, 2024 11:28
@Meinersbur Meinersbur closed this May 25, 2024
@Meinersbur
Copy link
Member Author

For me this entire change doesn't make sense.

Could you elaborate on why?

@PiotrZSL
Copy link
Member

For me this entire change doesn't make sense.

Could you elaborate on why?

This is basically names passed as FOLDER property are very generic. My main confusion where with clangTidyMiscModule library, but as this is semi-redundant. Then this change is +- fine.

@Meinersbur Meinersbur merged commit c87a7b3 into llvm:main May 25, 2024
8 checks passed
@Meinersbur
Copy link
Member Author

This is basically names passed as FOLDER property are very generic.

They are a little more specific than before (especially when considering Compiler-RT which put everything into "Compiler-RT/Misc"). Being more contextual would also require more maintenance, e.g. adding the right folder name for new artifacts. Only users of CMake's XCode or Visual Studio generators even see then, other would not even know something is off when this is not done. Hence the goal was to reduce maintenance, i.e. add_clang_library selects the folder name itself. It only has the type of the target available (here: library), so that's what it is restricted to.

Then this change is +- fine.

Thank you

Meinersbur added a commit that referenced this pull request May 27, 2024
This should have been removed from #89744 to address a review comment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants