[libcxx-commits] [PATCH] D59110: Restructure existing test suite to follow libc++ standard layout

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: test/CMakeLists.txt:21
 
-file(GLOB_RECURSE UNIT_TESTS "test_*.cpp")
+include_directories("${PROJECT_SOURCE_DIR}/include"
+                    ${CMAKE_CURRENT_LIST_DIR})
----------------
Please use `target_include_directories("pstl-${target}" PRIVATE "${CMAKE_CURRENT_LIST_DIR}")` below instead. `"${PROJECT_SOURCE_DIR}/include"` shouldn't be necessary at all since the tests link against `pstl::ParallelSTL`, which has `"${PROJECT_SOURCE_DIR}/include"` as an `INTERFACE` include directory.


================
Comment at: test/CMakeLists.txt:24
+
+add_definitions(-DPSTL_STANDALONE_TESTS)
+
----------------
Please use `target_compile_definitions("pstl-${target}" PRIVATE -DPSTL_STANDALONE_TESTS)` below instead. Setting global flags is frowned upon in modern CMake.


Repository:
  rPSTL pstl

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

https://reviews.llvm.org/D59110