[PATCH] D121838: [WIP] Generalize "check-all" umbrella targets, use for check-clang-tools

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Ran into problems caused by check-clang-tools not running clangd tests again, 
probably worth

In D121838#3433022 , @kbobyrev wrote:

> I think this is no longer [WIP] but rather review-ready, right?

It's both WIP and review-ready :-)
There are some mechanical cleanups left to do as mentioned in the comment 
above, but there's no point doing them if the idea is wrong.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121838/new/

https://reviews.llvm.org/D121838

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121838: [WIP] Generalize "check-all" umbrella targets, use for check-clang-tools

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

I think this is no longer [WIP] but rather review-ready, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121838/new/

https://reviews.llvm.org/D121838

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121838: [WIP] Generalize "check-all" umbrella targets, use for check-clang-tools

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 415931.
sammccall added a comment.

Fix LLVM_ADDITIONAL_TEST_{TARGETS,DEPENDS}


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121838/new/

https://reviews.llvm.org/D121838

Files:
  clang-tools-extra/CMakeLists.txt
  clang-tools-extra/test/CMakeLists.txt
  clang/CMakeLists.txt
  clang/bindings/python/tests/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/runtimes/CMakeLists.txt

Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -481,7 +481,7 @@
   endif()
 
   if(LLVM_INCLUDE_TESTS)
-set_property(GLOBAL APPEND PROPERTY LLVM_ADDITIONAL_TEST_DEPENDS runtimes-test-depends)
+set_property(GLOBAL APPEND PROPERTY LLVM_ALL_ADDITIONAL_TEST_DEPENDS runtimes-test-depends)
 
 set(RUNTIMES_TEST_DEPENDS
 FileCheck
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1822,17 +1822,63 @@
   set_target_properties(${target} PROPERTIES EXCLUDE_FROM_DEFAULT_BUILD ON)
 endfunction()
 
+# Convert a target name like check-clangd to a variable name like CLANG.
+function(umbrella_lit_testsuite_var target outvar)
+  if (NOT target MATCHES "^check-")
+message(FATAL_ERROR "umbrella lit suites must be check-*, not '${target}'")
+  endif()
+  string(SUBSTRING "${target}" 6 -1 var)
+  string(REPLACE "-" "_" var ${var})
+  string(TOUPPER "${var}" var)
+  set(${outvar} "${var}" PARENT_SCOPE)
+endfunction()
+
+# Start recording all lit test suites for a combined 'check-foo' target.
+# The recording continues until umbrella_lit_testsuite_end() creates the target.
+function(umbrella_lit_testsuite_begin target)
+  umbrella_lit_testsuite_var(${target} name)
+  set_property(GLOBAL APPEND PROPERTY LLVM_LIT_UMBRELLAS ${name})
+endfunction()
+
+# Create a combined 'check-foo' target for a set of related test suites.
+# It runs all suites added since the matching umbrella_lit_testsuite_end() call.
+# Tests marked EXCLUDE_FROM_CHECK_ALL are not gathered.
+function(umbrella_lit_testsuite_end target)
+  umbrella_lit_testsuite_var(${target} name)
+
+  get_property(testsuites GLOBAL PROPERTY LLVM_${name}_LIT_TESTSUITES)
+  get_property(params GLOBAL PROPERTY LLVM_${name}_LIT_PARAMS)
+  get_property(depends GLOBAL PROPERTY LLVM_${name}_LIT_DEPENDS)
+  get_property(extra_args GLOBAL PROPERTY LLVM_${name}_LIT_EXTRA_ARGS)
+  # Additional test targets are not gathered, but may be set externally.
+  get_property(additional_test_targets
+   GLOBAL PROPERTY LLVM_${name}_ADDITIONAL_TEST_TARGETS)
+
+  string(TOLOWER ${name} name)
+  add_lit_target(${target}
+"Running ${name} regression tests"
+${testsuites}
+PARAMS ${params}
+DEPENDS ${depends} ${additional_test_targets}
+ARGS ${extra_args}
+)
+endfunction()
+
 # A function to add a set of lit test suites to be driven through 'check-*' targets.
 function(add_lit_testsuite target comment)
   cmake_parse_arguments(ARG "EXCLUDE_FROM_CHECK_ALL" "" "PARAMS;DEPENDS;ARGS" ${ARGN})
 
   # EXCLUDE_FROM_ALL excludes the test ${target} out of check-all.
   if(NOT ARG_EXCLUDE_FROM_CHECK_ALL)
-# Register the testsuites, params and depends for the global check rule.
-set_property(GLOBAL APPEND PROPERTY LLVM_LIT_TESTSUITES ${ARG_UNPARSED_ARGUMENTS})
-set_property(GLOBAL APPEND PROPERTY LLVM_LIT_PARAMS ${ARG_PARAMS})
-set_property(GLOBAL APPEND PROPERTY LLVM_LIT_DEPENDS ${ARG_DEPENDS})
-set_property(GLOBAL APPEND PROPERTY LLVM_LIT_EXTRA_ARGS ${ARG_ARGS})
+get_property(gather_names GLOBAL PROPERTY LLVM_LIT_UMBRELLAS)
+set_property(GLOBAL APPEND PROPERTY LLVM_LIT_UMBRELLAS ${name})
+foreach(name ${gather_names})
+# Register the testsuites, params and depends for the umbrella check rule.
+  set_property(GLOBAL APPEND PROPERTY LLVM_${name}_LIT_TESTSUITES ${ARG_UNPARSED_ARGUMENTS})
+  set_property(GLOBAL APPEND PROPERTY LLVM_${name}_LIT_PARAMS ${ARG_PARAMS})
+  set_property(GLOBAL APPEND PROPERTY LLVM_${name}_LIT_DEPENDS ${ARG_DEPENDS})
+  set_property(GLOBAL APPEND PROPERTY LLVM_${name}_LIT_EXTRA_ARGS ${ARG_ARGS})
+endforeach()
   endif()
 
   # Produce a specific suffixed check rule.
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -1056,6 +1056,10 @@
   llvm_replace_compiler_option(CMAKE_CXX_FLAGS_RELEASE "-O3" "-O2")
 endif()
 
+if(LLVM_INCLUDE_TESTS)
+  umbrella_lit_testsuite_begin(check-all)
+endif()
+
 # Put this before tblgen. Else we have a circular dependence.
 add_subdirectory(lib/Demangle)
 add_subdirectory(lib/Support)
@@ -1127,25 +1131,12 @@
 add_subdirectory(utils/KillTheDoctor)
   endif()
 
-  # Add a 

[PATCH] D121838: [WIP] Generalize "check-all" umbrella targets, use for check-clang-tools

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This isn't ready for submission, there's some bits that touch the global 
variables in runtimes/, llvm/runtimes, and compiler-rt/test. Mostly to see the 
idea.

The main point is to allow check-clang-tools to cover the tests in 
clang-tools-extra without requiring them to be merged into a common test/ tree. 
(Thus enabling unmerging...)
It should also make it easy to add `check-all` targets to standalone builds of 
projects other than clang.

It probably makes sense to separate the behavior change for clang-tools-extra 
from the refactoring change to clang and llvm, combining them is mostly to make 
review clearer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121838/new/

https://reviews.llvm.org/D121838

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121838: [WIP] Generalize "check-all" umbrella targets, use for check-clang-tools

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added reviewers: aaron.ballman, hokein, thakis.
Herald added subscribers: usaxena95, kadircet, mgorny.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, ilya-biryukov.
Herald added projects: clang, LLVM, clang-tools-extra.

The mechanism behind "check-all" is recording params of add_lit_testsuite()
calls in global variables LLVM_LIT_*, and then creating an extra suite with
their union at the end.
This avoids composing the check-* targets directly, which doesn't work well.

We generalize this by allowing multiple families of variables LLVM_{name}_LIT_*:

  umbrella_lit_testsuite_begin(check-foo)
  ... test suites here will be added to LLVM_FOO_LIT_* variables ...
  umbrella_lit_testsuite_end(check-foo)

(This also moves some implementation muck out of {llvm,clang}/CMakeLists.txt

This patch also changes check-clang-tools to use be an umbrella test target,
which means the clangd and clang-pseudo tests are included in it, along with the
the other testsuites that already are (like check-clang-extra-clang-tidy).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121838

Files:
  clang-tools-extra/CMakeLists.txt
  clang-tools-extra/test/CMakeLists.txt
  clang/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake

Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1822,17 +1822,63 @@
   set_target_properties(${target} PROPERTIES EXCLUDE_FROM_DEFAULT_BUILD ON)
 endfunction()
 
+# Convert a target name like check-clangd to a variable name like CLANG.
+function(umbrella_lit_testsuite_var target outvar)
+  if (NOT target MATCHES "^check-")
+message(FATAL_ERROR "umbrella lit suites must be check-*, not '${target}'")
+  endif()
+  string(SUBSTRING "${target}" 6 -1 var)
+  string(REPLACE "-" "_" var ${var})
+  string(TOUPPER "${var}" var)
+  set(${outvar} "${var}" PARENT_SCOPE)
+endfunction()
+
+# Start recording all lit test suites for a combined 'check-foo' target.
+# The recording continues until umbrella_lit_testsuite_end() creates the target.
+function(umbrella_lit_testsuite_begin target)
+  umbrella_lit_testsuite_var(${target} name)
+  set_property(GLOBAL APPEND PROPERTY LLVM_LIT_UMBRELLAS ${name})
+endfunction()
+
+# Create a combined 'check-foo' target for a set of related test suites.
+# It runs all suites added since the matching umbrella_lit_testsuite_end() call.
+# Tests marked EXCLUDE_FROM_CHECK_ALL are not gathered.
+function(umbrella_lit_testsuite_end target)
+  umbrella_lit_testsuite_var(${target} name)
+
+  get_property(testsuites GLOBAL PROPERTY LLVM_${name}_LIT_TESTSUITES)
+  get_property(params GLOBAL PROPERTY LLVM_${name}_LIT_PARAMS)
+  get_property(depends GLOBAL PROPERTY LLVM_${name}_LIT_DEPENDS)
+  get_property(extra_args GLOBAL PROPERTY LLVM_${name}_LIT_EXTRA_ARGS)
+  # Additional test targets are not gathered, but may be set externally.
+  get_property(additional_test_targets
+   GLOBAL PROPERTY LLVM_${name}_ADDITIONAL_TEST_TARGETS)
+
+  string(TOLOWER ${name} name)
+  add_lit_target(${target}
+"Running ${name} regression tests"
+${testsuites}
+PARAMS ${params}
+DEPENDS ${depends} ${additional_test_targets}
+ARGS ${extra_args}
+)
+endfunction()
+
 # A function to add a set of lit test suites to be driven through 'check-*' targets.
 function(add_lit_testsuite target comment)
   cmake_parse_arguments(ARG "EXCLUDE_FROM_CHECK_ALL" "" "PARAMS;DEPENDS;ARGS" ${ARGN})
 
   # EXCLUDE_FROM_ALL excludes the test ${target} out of check-all.
   if(NOT ARG_EXCLUDE_FROM_CHECK_ALL)
-# Register the testsuites, params and depends for the global check rule.
-set_property(GLOBAL APPEND PROPERTY LLVM_LIT_TESTSUITES ${ARG_UNPARSED_ARGUMENTS})
-set_property(GLOBAL APPEND PROPERTY LLVM_LIT_PARAMS ${ARG_PARAMS})
-set_property(GLOBAL APPEND PROPERTY LLVM_LIT_DEPENDS ${ARG_DEPENDS})
-set_property(GLOBAL APPEND PROPERTY LLVM_LIT_EXTRA_ARGS ${ARG_ARGS})
+get_property(gather_names GLOBAL PROPERTY LLVM_LIT_UMBRELLAS)
+set_property(GLOBAL APPEND PROPERTY LLVM_LIT_UMBRELLAS ${name})
+foreach(name ${gather_names})
+# Register the testsuites, params and depends for the umbrella check rule.
+  set_property(GLOBAL APPEND PROPERTY LLVM_${name}_LIT_TESTSUITES ${ARG_UNPARSED_ARGUMENTS})
+  set_property(GLOBAL APPEND PROPERTY LLVM_${name}_LIT_PARAMS ${ARG_PARAMS})
+  set_property(GLOBAL APPEND PROPERTY LLVM_${name}_LIT_DEPENDS ${ARG_DEPENDS})
+  set_property(GLOBAL APPEND PROPERTY LLVM_${name}_LIT_EXTRA_ARGS ${ARG_ARGS})
+endforeach()
   endif()
 
   # Produce a specific suffixed check rule.
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -1056,6 +1056,10 @@