[SYCL][NFC] Re-enable, fix and upgrade headers testing (#16117)

We have a suite to check that every header contains enough `#include`
directives and forward declarations so that it can be included
standalone and used. However, that suite got accidentally disabled some
time ago in #14879

This PR re-enabled the suite and fixed all issues it detected.

Besides that, the suite is upgraded:
- it now scans source directory, meaning that it won't complain about
headers which were removed from the codebase, but left in build folder
- it is now possible to have add a custom test instead of auto-generated
one: this is useful when you want to trigger certain template
instantiations to make sure that all code paths are covered

Author: AlexeySachkov

sycl/include/sycl/detail/id_queries_fit_in_int.hpp

@@ -23,6 +23,8 @@
 #ifndef __SYCL_DEVICE_ONLY__
 
 #include <sycl/exception.hpp>
+#include <sycl/nd_range.hpp>
+#include <sycl/range.hpp>
 
 #include <limits>
 #include <type_traits>

sycl/include/sycl/ext/oneapi/backend/hip.hpp

@@ -8,7 +8,7 @@
 
 #pragma once
 
-#include <sycl/backend_types.hpp>
+#include <sycl/backend.hpp>
 #include <sycl/detail/backend_traits_hip.hpp>
 
 namespace sycl {

sycl/include/sycl/ext/oneapi/experimental/raw_kernel_arg.hpp

@@ -14,6 +14,9 @@ namespace sycl {
 inline namespace _V1 {
 
 class handler;
+namespace detail {
+class dynamic_parameter_impl;
+}
 
 namespace ext::oneapi::experimental {
 

sycl/include/sycl/ext/oneapi/experimental/virtual_functions.hpp

@@ -1,8 +1,11 @@
 #pragma once
 
 #include <sycl/ext/oneapi/properties/property.hpp>
+#include <sycl/ext/oneapi/properties/property_utils.hpp>
 #include <sycl/ext/oneapi/properties/property_value.hpp>
 
+#include <utility>
+
 namespace sycl {
 inline namespace _V1 {
 namespace ext::oneapi::experimental {

sycl/include/sycl/ext/oneapi/experimental/work_group_memory.hpp

@@ -7,10 +7,18 @@
 
 #pragma once
 
+#include <sycl/access/access.hpp>
+#include <sycl/detail/defines.hpp>
+#include <sycl/ext/oneapi/properties/properties.hpp>
+#include <sycl/multi_ptr.hpp>
+
+#include <cstddef>
 #include <type_traits>
 
 namespace sycl {
 inline namespace _V1 {
+class handler;
+
 namespace detail {
 template <typename T> struct is_unbounded_array : std::false_type {};
 
@@ -37,8 +45,10 @@ namespace ext::oneapi::experimental {
 
 struct indeterminate_t {};
 inline constexpr indeterminate_t indeterminate;
-
 template <typename DataT, typename PropertyListT = empty_properties_t>
+class work_group_memory;
+
+template <typename DataT, typename PropertyListT>
 class __SYCL_SPECIAL_CLASS __SYCL_TYPE(work_group_memory) work_group_memory
     : sycl::detail::work_group_memory_impl {
 public:

sycl/test/format.py

@@ -5,26 +5,57 @@
 import os
 import re
 
-SUFFIXES = {".hpp"}
-
-
 class SYCLHeadersTest(lit.formats.TestFormat):
     def getTestsForPath(
         self, testSuite, path_in_suite, filepath, litConfig, localConfig
     ):
+        # path_in_suite is a tuple like:
+        #   ('self-contained-headers', 'path/to', 'file.hpp')
+        test_path = testSuite.getSourcePath(path_in_suite) + ".cpp"
+        if os.path.exists(test_path):
+            # We have a dedicated special test for a header, let's use a file
+            # from the suite itself
+
+            # None is a special value we use to distinguish those two cases
+            filepath = None
+            # The actual file has .cpp extension as every other test
+            path_in_suite = path_in_suite[:-1] + (path_in_suite[-1] + ".cpp",)
+        else:
+            # We don't have a dedicated special test for a header, therefore we
+            # fallback to a generalized version of it
+
+            # SYCL headers may depend on some generated files and therefore we
+            # use headers from the build folder for testing
+            filepath = os.path.join(localConfig.sycl_include, *path_in_suite[1:])
+
         yield lit.Test.Test(testSuite, path_in_suite, localConfig, file_path=filepath)
 
     def getTestsInDirectory(self, testSuite, path_in_suite, litConfig, localConfig):
-        # We traverse build/sycl/include/sycl directory
-        source_path = os.path.join(localConfig.sycl_include, "sycl")
+        # To respect SYCL_LIB_DUMPS_ONLY mode
+        if ".cpp" not in localConfig.suffixes:
+            return
+
+        # As we add more files and folders into 'self-contained-headers', this
+        # method will be recursivelly called for them by lit discovery.
+        # However, we don't use the test folder as the source of tests but
+        # instead we use SYCL_SOURCE_DIR/include/sycl directory.
+        # Therefore, we exit early from here if `path_in_suite` conatins more
+        # than one element
+        assert path_in_suite[0] == "self-contained-headers"
+        if len(path_in_suite) > 1:
+            return
+
+        source_path = os.path.join(localConfig.sycl_include_source_dir, "sycl")
 
         # Optional filter can be passed through command line options
         headers_filter = localConfig.sycl_headers_filter
         for dirpath, _, filenames in os.walk(source_path):
-            relative_dirpath = dirpath[len(localConfig.sycl_include) + 1 :]
+            relative_dirpath = dirpath[len(localConfig.sycl_include_source_dir) + 1 :]
             for filename in filenames:
                 suffix = os.path.splitext(filename)[1]
-                if suffix not in SUFFIXES or suffix not in litConfig.suffixes:
+                # We only look at actual header files and not at their .inc/.def
+                # components
+                if suffix != ".hpp":
                     continue
                 filepath = os.path.join(dirpath, filename)
 
@@ -46,6 +77,18 @@ def getTestsInDirectory(self, testSuite, path_in_suite, litConfig, localConfig):
                     yield t
 
     def execute(self, test, litConfig):
+        if test.file_path is None:
+            # It means that we have a special test case for a header and we need
+            # to execute it as a regular lit sh test
+            return lit.TestRunner.executeShTest(
+                test,
+                litConfig,
+                False,  # execute_external
+                [],  # extra_substitutions
+                [],  # preamble_commands
+            )
+
+        # Otherwise we generate the test on the fly
         command = [
             test.config.clang,
             "-fsycl",

sycl/test/lit.site.cfg.py.in

@@ -9,6 +9,7 @@ config.sycl_tools_dir = lit_config.params.get('SYCL_TOOLS_DIR', "@LLVM_TOOLS_DIR
 config.sycl_include = lit_config.params.get('SYCL_INCLUDE', "@SYCL_INCLUDE@")
 config.sycl_obj_root = "@SYCL_BINARY_DIR@"
 config.sycl_source_dir = "@SYCL_SOURCE_DIR@/source"
+config.sycl_include_source_dir = "@SYCL_SOURCE_DIR@/include"
 config.sycl_libs_dir = lit_config.params.get('SYCL_LIBS_DIR', "@LLVM_LIBS_DIR@")
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.host_triple = "@LLVM_HOST_TRIPLE@"

sycl/test/self-contained-headers/README.md

@@ -14,23 +14,38 @@ still be sure that we haven't accidentally removed a necessary `#include`.
 meaning that any warnings coming out of them may be turned into errors and will
 affect test results. This is considered as an extra feature of the suite.
 
+**One more note:** due to templated nature of SYCL headers, not every code path
+may be instantiated by a mere `#include` and therefore not every dependency will
+be highlighted by a simple test. To overcome this, there is an ability to write
+dedicated tests for certain headers which are more exhaustive than a simple
+`#include`, see more details below.
+
 ## Implementation
 
 There was a couple of iterations on the suite design and its current shape
 features the following:
-- each header in `build/include/sycl` is checked as a separate test
-- each such test is generated on the fly dynamically during LIT discovery phase
+- each header in `build/include/sycl` is checked as a separate test, unless:
+  - it doesn't exists in `source/include/sycl`, meaning that it is likely
+    removed from the codebase, but still resides in `build/` directory
+  - **TODO:** we also have some auto-generated headers which could be skipped
+    this way, we need to consider a mechanism to handle them as well
+  - **TODO:** presence of outdated headers in `build` directory should also be
+    detected, or otherwise it can lead to compilation issues being hidden in
+    local setup
+- each such test is generated on the fly dynamically during LIT discovery phase,
+  unless:
+  - there is a special/dedicated test for a header, more details below
 
 That is done to allow for massive parallelism and keep those tests small and
 quick.
 
-Absolute most of the magic is happenning within
+Absolute most of the magic is happening within
 [`sycl/test/format.py`](/sycl/test/format.py): we define a custom test format in
 there which overrides standard discovery and test execution rules.
 
 ## How to use and maintain
 
-Those tests are part of `check-sycl` target and you can pass a regexp acepted
+Those tests are part of `check-sycl` target and you can pass a regexp accepted
 by Python's `re` package as `SYCL_HEADERS_FILTER` parameter to LIT to filter
 which headers you would like to see checked (only those that match the passed
 regexp will be used to generate tests).
@@ -47,11 +62,38 @@ Documentation for Python's regexp can be found [here][python-3-re].
 
 [python-3-re]: https://docs.python.org/3/library/re.html#regular-expression-syntax
 
-Since there are no dedicated files for each test, `XFAIL`ing them using regular
-method is impossible, but it is still supported. To do so, open
+Since there are no dedicated files for auto-generated tests, `XFAIL`ing them
+using regular method is impossible, but it is still supported. To do so, open
 [the local config](/sycl/test/self-contained-headers/lit.local.cfg) and modify
 list of files which should be treated as expected to fail.
 
+### Special tests
+
+As noted above, to truly ensure that SYCL headers are self-contained, we need
+not only include them, but also use them
+(read: instantiate all classes and methods).
+
+To support that, for every SYCL header we have in `source/include/sycl` the tool
+first checks if there is a corresponding test file in
+`source/test/self-contained-headers` and if so, it is used instead of an
+auto-generated one.
+
+Those special tests should be named and located in certain place to be detected,
+or otherwise they will be ignored. For a header
+`source/include/sycl/path/to/header.hpp` its special test should be placed under
+`source/test/sycl/self-contained-headers/sycl/path/to/header.hpp.cpp`.
+
+Note a few things: directory structure should exactly match, the filename should
+be the same as the header file name, but with `.cpp` extension added on top of
+it.
+
+Those special tests will be treated as any other regular Sh-based tests, i.e.
+you should write your regular `RUN` lines in there. It is expected that those
+tests will run a compilation under `-fsyntax-only` mode and verify absence of
+any compilation errors or warnings through `-Xclang -verify` mechanism.
+
+Special tests can be `XFAIL`-ed using a regular LIT mechanism.
+
 ## Known issues and quirks
 
 ### To launch the suite directly, use `LIT_FILTER` env variable
@@ -70,14 +112,7 @@ Instead, the following approach should be used:
 LIT_FILTER='self-contained-headers' llvm-lit sycl/test
 ```
 
-### Old legacy files in build/ area are still checked
-
-The custom discovery script uses `build/include/sycl/` folder contents to
-generate tests for each header it finds there. It means that if some header was
-removed from the codebase, it may still be present in `build` folder unless
-some cleanup is performed.
-
-### No OS-specific `XFAIL` mechanism is implemented
+### No OS-specific `XFAIL` mechanism is implemented for auto-generated tests
 
 `XFAIL` mechanism mentioned in "How to use and maintain" section does not
 support marking a test as expected to fail only in certain environment, which

sycl/test/self-contained-headers/sycl/handler.hpp.cpp

@@ -0,0 +1,16 @@
+// RUN: %clangxx -fsycl -fsyntax-only -Xclang -verify %s
+// expected-no-diagnostics
+//
+// The purpose of this test is to ensure that the header containing
+// sycl::handler class definition is self-contained, i.e. we can use handler
+// and no extra headers are needed.
+//
+// TODO: the test should be expanded to use various methods of the class. Due
+// to their template nature we may not test all code paths until we trigger
+// instantiation of a corresponding method.
+
+#include <sycl/handler.hpp>
+
+class kernel_name;
+
+void foo(sycl::handler &h) {}