From d8d75e79247eb962b745c84d4981dab8a0990718 Mon Sep 17 00:00:00 2001 From: sbstndb/sbstndbs <30315405+sbstndb@users.noreply.github.com> Date: Thu, 3 Apr 2025 10:45:24 +0200 Subject: [PATCH 1/9] Update CMakeLists.txt --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c7ec920c9..ded183248 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -194,7 +194,7 @@ target_include_directories(xtensor INTERFACE $ $) -target_compile_features(xtensor INTERFACE cxx_std_17) +target_compile_features(xtensor INTERFACE cxx_std_20) target_link_libraries(xtensor INTERFACE xtl) From 46889a429d767035e5819d8fa70f0f8e86ca41ed Mon Sep 17 00:00:00 2001 From: sbstndb/sbstndbs Date: Fri, 4 Apr 2025 14:04:34 +0200 Subject: [PATCH 2/9] feat: Add c++23 option - Previously, we had the option of activating c++20. - But as we defined c++20 as default, I suggest to replace this option as c++23. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ded183248..f2a9fdf12 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -205,7 +205,7 @@ OPTION(BUILD_TESTS "xtensor test suite" OFF) OPTION(BUILD_BENCHMARK "xtensor benchmark" OFF) OPTION(DOWNLOAD_GBENCHMARK "download google benchmark and build from source" ON) OPTION(DEFAULT_COLUMN_MAJOR "set default layout to column major" OFF) -OPTION(CPP20 "enables C++20 (experimental)" OFF) +OPTION(CPP23 "enables C++23 (experimental)" OFF) OPTION(XTENSOR_DISABLE_EXCEPTIONS "Disable C++ exceptions" OFF) OPTION(DISABLE_MSVC_ITERATOR_CHECK "Disable the MVSC iterator check" ON) From 7365d809ffa66bc49f7a389f096a01246d26cfda Mon Sep 17 00:00:00 2001 From: sbstndb/sbstndbs Date: Fri, 4 Apr 2025 14:40:44 +0200 Subject: [PATCH 3/9] feat: Use c++20 in tests - Previously, tests was in c++17 - We want to add c++20 by default, so I modify the test cmake - The tests passed locally --- test/CMakeLists.txt | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index cc2f9281a..5d5da0e7f 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -34,27 +34,27 @@ string(TOUPPER "${CMAKE_BUILD_TYPE}" U_CMAKE_BUILD_TYPE) include(set_compiler_flag.cmake) -if(CPP20) - # User requested C++17, but compiler might not oblige. +if(CPP23) + # User requested C++20, but compiler might not oblige. set_compiler_flag( _cxx_std_flag CXX - "-std=c++2a" # this should work with GNU, Intel, PGI - "/std:c++20" # this should work with MSVC + "-std=c++23" # this should work with GNU, Intel, PGI + "/std:c++23" # this should work with MSVC ) if(_cxx_std_flag) - message(STATUS "Building with C++20") + message(STATUS "Building with C++23") endif() else() set_compiler_flag( _cxx_std_flag CXX REQUIRED - "-std=c++17" # this should work with GNU, Intel, PGI - "/std:c++17" # this should work with MSVC + "-std=c++20" # this should work with GNU, Intel, PGI + "/std:c++20" # this should work with MSVC ) - message(STATUS "Building with C++17") + message(STATUS "Building with C++20") endif() if(NOT _cxx_std_flag) - message(FATAL_ERROR "xtensor needs a C++17-compliant compiler.") + message(FATAL_ERROR "xtensor needs a C++20-compliant compiler.") endif() OPTION(XTENSOR_ENABLE_WERROR "Turn on -Werror" OFF) From 2f2e5a8079f5728bf47302f315e6121c39bb85db Mon Sep 17 00:00:00 2001 From: sbstndb/sbstndbs Date: Fri, 4 Apr 2025 14:51:05 +0200 Subject: [PATCH 4/9] fix: type --- test/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 5d5da0e7f..363595b5e 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -35,7 +35,7 @@ string(TOUPPER "${CMAKE_BUILD_TYPE}" U_CMAKE_BUILD_TYPE) include(set_compiler_flag.cmake) if(CPP23) - # User requested C++20, but compiler might not oblige. + # User requested C++23, but compiler might not oblige. set_compiler_flag( _cxx_std_flag CXX "-std=c++23" # this should work with GNU, Intel, PGI From 7a0d20eadc2919eefdd9e631cbc0a908b2cdd560 Mon Sep 17 00:00:00 2001 From: sbstndb/sbstndbs Date: Fri, 4 Apr 2025 16:49:10 +0200 Subject: [PATCH 5/9] fix: Add MSVC flag - Previously, The MSVC version was not well recognized --> Hence, there was some bugy macro specific to MSVC/c++20 - This Commit resolve the issue and I am now able to compile the problematic testcase - Question : SHould we add this flag not at the tets level but at the main CMakeFiles level ? --- test/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 363595b5e..884de312b 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -74,7 +74,7 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "GNU" OR (CMAKE_CXX_COMPILER_ID MATCHES "Intel" set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror -DSKIP_ON_WERROR") endif() elseif(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${_cxx_std_flag} /MP /bigobj") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${_cxx_std_flag} /Zc:__cplusplus /MP /bigobj") set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /MANIFEST:NO") add_definitions(-D_CRT_SECURE_NO_WARNINGS) add_definitions(-D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING) From 93be135f571e48ce338c5cc6ac923c8b6e1d3c9c Mon Sep 17 00:00:00 2001 From: sbstndb/sbstndbs Date: Mon, 7 Apr 2025 13:23:37 +0200 Subject: [PATCH 6/9] fix: enforce specialization - Previously, on MSVC, in c++20, the empty() function was incorrectly deduced - Hence the test_xbuilder no longer pass - I suggest enforce the right specialization by specifying the last template parameter --- test/test_xbuilder.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_xbuilder.cpp b/test/test_xbuilder.cpp index fd69922d0..6e658ef41 100644 --- a/test/test_xbuilder.cpp +++ b/test/test_xbuilder.cpp @@ -676,7 +676,8 @@ namespace xt xt::dynamic_shape sd = {3, 2, 1}; auto ed1 = empty(sd); - auto ed2 = empty(dynamic_shape({3, 3, 3})); + using ShapeType = xt::dynamic_shape; + auto ed2 = empty(ShapeType({3, 3, 3})); auto ed3 = empty(std::vector({3, 3, 3})); b = std::is_same>::value; EXPECT_TRUE(b); From 0d587595405ca67cf5ac717eca81b89a2698eebb Mon Sep 17 00:00:00 2001 From: sbstndb/sbstndbs Date: Mon, 7 Apr 2025 13:27:39 +0200 Subject: [PATCH 7/9] fix: resolve precommit issue --- test/test_xbuilder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_xbuilder.cpp b/test/test_xbuilder.cpp index 6e658ef41..8eb024ccc 100644 --- a/test/test_xbuilder.cpp +++ b/test/test_xbuilder.cpp @@ -676,7 +676,7 @@ namespace xt xt::dynamic_shape sd = {3, 2, 1}; auto ed1 = empty(sd); - using ShapeType = xt::dynamic_shape; + using ShapeType = xt::dynamic_shape; auto ed2 = empty(ShapeType({3, 3, 3})); auto ed3 = empty(std::vector({3, 3, 3})); b = std::is_same>::value; From d615424199b5044cf641962ac06775ace653cfdb Mon Sep 17 00:00:00 2001 From: sbstndb/sbstndbs Date: Tue, 8 Apr 2025 11:19:34 +0200 Subject: [PATCH 8/9] fix: Workaround to fix MSVC test --- test/test_xbuilder.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/test_xbuilder.cpp b/test/test_xbuilder.cpp index 8eb024ccc..48f94b3cd 100644 --- a/test/test_xbuilder.cpp +++ b/test/test_xbuilder.cpp @@ -676,8 +676,12 @@ namespace xt xt::dynamic_shape sd = {3, 2, 1}; auto ed1 = empty(sd); +#if defined(_MSVC_LANG) using ShapeType = xt::dynamic_shape; auto ed2 = empty(ShapeType({3, 3, 3})); +#else + auto ed2 = empty(xt::dynamic_shape({3, 3, 3})); +#endif auto ed3 = empty(std::vector({3, 3, 3})); b = std::is_same>::value; EXPECT_TRUE(b); From 311a4b2786239842bfb9b007515d936189cbaa56 Mon Sep 17 00:00:00 2001 From: sbstndb/sbstndbs Date: Tue, 8 Apr 2025 11:40:22 +0200 Subject: [PATCH 9/9] doc: add comment --- test/test_xbuilder.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_xbuilder.cpp b/test/test_xbuilder.cpp index 48f94b3cd..73d952b6f 100644 --- a/test/test_xbuilder.cpp +++ b/test/test_xbuilder.cpp @@ -676,7 +676,8 @@ namespace xt xt::dynamic_shape sd = {3, 2, 1}; auto ed1 = empty(sd); -#if defined(_MSVC_LANG) +// TODO : The ed2 expression do not work on MSVC due to bad deduction since cpp20. We need to fix it for MSVC. +#if defined(_MSVC_LANG) using ShapeType = xt::dynamic_shape; auto ed2 = empty(ShapeType({3, 3, 3})); #else