From 6f5b360681b01fa8db3d44b55119af9163589b6c Mon Sep 17 00:00:00 2001 From: Alexey Ochapov Date: Mon, 13 Dec 2021 02:24:04 +0300 Subject: [PATCH 1/4] rewrite compile-error-test to use non-header-only library --- test/CMakeLists.txt | 4 +- test/compile-error-test/CMakeLists.txt | 167 ++++++++++++++++++------- 2 files changed, 122 insertions(+), 49 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c75c4f28389e..10dcd17c0cea 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -168,9 +168,9 @@ if (FMT_PEDANTIC) --build-makeprogram ${CMAKE_MAKE_PROGRAM} --build-options "-DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}" + "-DCMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}" "-DCMAKE_CXX_STANDARD=${CMAKE_CXX_STANDARD}" - "-DCXX_STANDARD_FLAG=${CXX_STANDARD_FLAG}" - "-DPEDANTIC_COMPILE_FLAGS=${PEDANTIC_COMPILE_FLAGS}" + "-DFMT_DIR=${CMAKE_SOURCE_DIR}" "-DSUPPORTS_USER_DEFINED_LITERALS=${SUPPORTS_USER_DEFINED_LITERALS}") endif () diff --git a/test/compile-error-test/CMakeLists.txt b/test/compile-error-test/CMakeLists.txt index 44bbb1aba020..5a40094b1fc0 100644 --- a/test/compile-error-test/CMakeLists.txt +++ b/test/compile-error-test/CMakeLists.txt @@ -1,77 +1,145 @@ # Test if compile errors are produced where necessary. cmake_minimum_required(VERSION 3.1...3.18) +project(compile-error-test CXX) -include(CheckCXXSourceCompiles) -include(CheckCXXCompilerFlag) +set(fmt_headers " + #include + #include +") -set(CMAKE_REQUIRED_INCLUDES ${CMAKE_CURRENT_SOURCE_DIR}/../../include) -set(CMAKE_REQUIRED_FLAGS ${CXX_STANDARD_FLAG} ${PEDANTIC_COMPILE_FLAGS}) +set(error_test_names "") +set(non_error_test_content "") -function (generate_source result fragment) - set(${result} " - #define FMT_HEADER_ONLY 1 - #include \"fmt/format.h\" - int main() { - ${fragment} - } - " PARENT_SCOPE) -endfunction () +# For error tests (we expect them to produce compilation error): +# * adds a name of test into `error_test_names` list +# * generates a single source file (with the same name) for each test +# For non-error tests (we expect them to compile successfully): +# * adds a code segment as separate function to `non_error_test_content` +function (expect_compile name code_fragment) + cmake_parse_arguments(EXPECT_COMPILE "ERROR" "" "" ${ARGN}) + string(MAKE_C_IDENTIFIER "${name}" test_name) -function (expect_compile code) - generate_source(source "${code}") - check_cxx_source_compiles("${source}" compiles) - if (NOT compiles) - set(error_msg "Compile error for: ${code}") - endif () - # Unset the CMake cache variable compiles. Otherwise the compile test will - # just use cached information next time it runs. - unset(compiles CACHE) - if (error_msg) - message(FATAL_ERROR ${error_msg}) - endif () + if (EXPECT_COMPILE_ERROR) + file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/test/${test_name}.cc" " + ${fmt_headers} + void ${test_name}() { + ${code_fragment} + } + ") + set(error_test_names_copy "${error_test_names}") + list(APPEND error_test_names_copy "${test_name}") + set(error_test_names "${error_test_names_copy}" PARENT_SCOPE) + else() + set(non_error_test_content " + ${non_error_test_content} + void ${test_name}() { + ${code_fragment} + }" PARENT_SCOPE) + endif() endfunction () -function (expect_compile_error code) - generate_source(source "${code}") - check_cxx_source_compiles("${source}" compiles) - if (compiles) - set(error_msg "No compile error for: ${code}") - endif () - # Unset the CMake cache variable compiles. Otherwise the compile test will - # just use cached information next time it runs. - unset(compiles CACHE) - if (error_msg) - message(FATAL_ERROR ${error_msg}) +# Generates a source file for non-error test with `non_error_test_content` and +# CMake project file with all error and single non-error test targets. +function (run_tests) + set(cmake_targets "") + foreach(test_name IN LISTS error_test_names) + set(cmake_targets " + ${cmake_targets} + add_library(test-${test_name} ${test_name}.cc) + target_link_libraries(test-${test_name} PRIVATE fmt::fmt) + ") + endforeach() + + file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/test/non_error_test.cc" " + ${fmt_headers} + ${non_error_test_content} + ") + set(cmake_targets " + ${cmake_targets} + add_library(non-error-test non_error_test.cc) + target_link_libraries(non-error-test PRIVATE fmt::fmt) + ") + + file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/test/CMakeLists.txt" " + cmake_minimum_required(VERSION 3.1...3.18) + project(tests CXX) + add_subdirectory(${FMT_DIR} fmt) + ${cmake_targets} + ") + + set(build_directory "${CMAKE_CURRENT_BINARY_DIR}/test/build") + file(MAKE_DIRECTORY "${build_directory}") + execute_process( + COMMAND + "${CMAKE_COMMAND}" + "-DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}" + "-DCMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}" + "-DCMAKE_CXX_STANDARD=${CMAKE_CXX_STANDARD}" + "-DCMAKE_GENERATOR=${CMAKE_GENERATOR}" + "-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}" + "-DFMT_DIR=${FMT_DIR}" + "${CMAKE_CURRENT_BINARY_DIR}/test" + WORKING_DIRECTORY "${build_directory}" + RESULT_VARIABLE result_var + OUTPUT_VARIABLE output_var + ERROR_VARIABLE output_var) + if (NOT result_var EQUAL 0) + message(FATAL_ERROR "Unable to configure:\n${output_var}") + endif() + + foreach(test_name IN LISTS error_test_names) + execute_process( + COMMAND + "${CMAKE_COMMAND}" --build "${build_directory}" --target "test-${test_name}" + WORKING_DIRECTORY "${build_directory}" + RESULT_VARIABLE result_var + OUTPUT_VARIABLE output_var + ERROR_QUIET) + if (result_var EQUAL 0) + message(SEND_ERROR "No compile error for \"${test_name}\":\n${output_var}") + endif () + endforeach() + + execute_process( + COMMAND + "${CMAKE_COMMAND}" --build "${build_directory}" --target "non-error-test" + WORKING_DIRECTORY "${build_directory}" + RESULT_VARIABLE result_var + OUTPUT_VARIABLE output_var + ERROR_VARIABLE output_var) + if (NOT result_var EQUAL 0) + message(SEND_ERROR "Compile error for combined non-error test:\n${output_var}") endif () endfunction () + # check if the source file skeleton compiles -expect_compile("") +expect_compile(check "") # Formatting a wide character with a narrow format string is forbidden. -expect_compile_error("fmt::format(\"{}\", L'a');") +expect_compile(wide-character-narrow-format-string "fmt::format(\"{}\", L'a');" ERROR) # Formatting a wide string with a narrow format string is forbidden. -expect_compile_error("fmt::format(\"{}\", L\"foo\");") +expect_compile(wide-string-narrow-format-string "fmt::format(\"{}\", L\"foo\");" ERROR) # Formatting a narrow string with a wide format string is forbidden because # mixing UTF-8 with UTF-16/32 can result in an invalid output. -expect_compile_error("fmt::format(L\"{}\", \"foo\");") +expect_compile(narrow-string-wide-format-string "fmt::format(L\"{}\", \"foo\");" ERROR) # Formatting a wide string with a narrow format string is forbidden. -expect_compile_error(" +expect_compile(cast-to-string " struct S { operator std::string() const { return std::string(); } }; fmt::format(\"{}\", S()); -") +" ERROR) # Formatting a function -expect_compile_error(" +expect_compile(format-function " void (*f)(); fmt::format(\"{}\", f); -") +" ERROR) # Make sure that compiler features detected in the header # match the features detected in CMake. @@ -80,6 +148,11 @@ if (SUPPORTS_USER_DEFINED_LITERALS) else () set(supports_udl 0) endif () -expect_compile("#if FMT_USE_USER_DEFINED_LITERALS != ${supports_udl} - # error - #endif") +expect_compile(udl-check " + #if FMT_USE_USER_DEFINED_LITERALS != ${supports_udl} + # error + #endif +") + +# Run all tests +run_tests() From 1f68d6f0b499080a32c649a33425a6ae7caa895f Mon Sep 17 00:00:00 2001 From: Alexey Ochapov Date: Thu, 23 Dec 2021 23:29:07 +0300 Subject: [PATCH 2/4] disable compile-error-test on Windows since it takes too much time to complete, similar to other tests with additional CMake invocation --- test/CMakeLists.txt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 10dcd17c0cea..dac123125345 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -159,7 +159,11 @@ if (FMT_PEDANTIC) nolocale-test PRIVATE ${PROJECT_SOURCE_DIR}/include) target_compile_definitions( nolocale-test PRIVATE FMT_STATIC_THOUSANDS_SEPARATOR=1) +endif () +# These tests are disabled on Windows because they take too long. +if (FMT_PEDANTIC AND NOT WIN32) + # Test if incorrect API usages produce compilation error. add_test(compile-error-test ${CMAKE_CTEST_COMMAND} --build-and-test "${CMAKE_CURRENT_SOURCE_DIR}/compile-error-test" @@ -172,10 +176,7 @@ if (FMT_PEDANTIC) "-DCMAKE_CXX_STANDARD=${CMAKE_CXX_STANDARD}" "-DFMT_DIR=${CMAKE_SOURCE_DIR}" "-DSUPPORTS_USER_DEFINED_LITERALS=${SUPPORTS_USER_DEFINED_LITERALS}") -endif () -# These tests are disabled on Windows because they take too long. -if (FMT_PEDANTIC AND NOT WIN32) # Test if the targets are found from the build directory. add_test(find-package-test ${CMAKE_CTEST_COMMAND} -C ${CMAKE_BUILD_TYPE} From 735cf3056d91e1a128159a2806adf94acfb845de Mon Sep 17 00:00:00 2001 From: Alexey Ochapov Date: Mon, 13 Dec 2021 02:44:52 +0300 Subject: [PATCH 3/4] add reverse tests to compile-error-test to make sure that error tests do not fail because they become outdated --- test/compile-error-test/CMakeLists.txt | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/test/compile-error-test/CMakeLists.txt b/test/compile-error-test/CMakeLists.txt index 5a40094b1fc0..5b0a172ffd6e 100644 --- a/test/compile-error-test/CMakeLists.txt +++ b/test/compile-error-test/CMakeLists.txt @@ -116,19 +116,28 @@ endfunction () # check if the source file skeleton compiles expect_compile(check "") +expect_compile(check-error "compilation_error" ERROR) # Formatting a wide character with a narrow format string is forbidden. -expect_compile(wide-character-narrow-format-string "fmt::format(\"{}\", L'a');" ERROR) +expect_compile(wide-character-narrow-format-string "fmt::format(L\"{}\", L'a');") +expect_compile(wide-character-narrow-format-string-error "fmt::format(\"{}\", L'a');" ERROR) # Formatting a wide string with a narrow format string is forbidden. -expect_compile(wide-string-narrow-format-string "fmt::format(\"{}\", L\"foo\");" ERROR) +expect_compile(wide-string-narrow-format-string "fmt::format(L\"{}\", L\"foo\");") +expect_compile(wide-string-narrow-format-string-error "fmt::format(\"{}\", L\"foo\");" ERROR) # Formatting a narrow string with a wide format string is forbidden because # mixing UTF-8 with UTF-16/32 can result in an invalid output. -expect_compile(narrow-string-wide-format-string "fmt::format(L\"{}\", \"foo\");" ERROR) +expect_compile(narrow-string-wide-format-string "fmt::format(L\"{}\", L\"foo\");") +expect_compile(narrow-string-wide-format-string-error "fmt::format(L\"{}\", \"foo\");" ERROR) -# Formatting a wide string with a narrow format string is forbidden. expect_compile(cast-to-string " + struct S { + operator std::string() const { return std::string(); } + }; + fmt::format(\"{}\", std::string(S())); +") +expect_compile(cast-to-string-error " struct S { operator std::string() const { return std::string(); } }; @@ -137,6 +146,10 @@ expect_compile(cast-to-string " # Formatting a function expect_compile(format-function " + void (*f)(); + fmt::format(\"{}\", fmt::ptr(f)); +") +expect_compile(format-function-error " void (*f)(); fmt::format(\"{}\", f); " ERROR) From 8c86bfcbfd7e9cf62e8dba9ac34f6e4dae5868cc Mon Sep 17 00:00:00 2001 From: Alexey Ochapov Date: Thu, 23 Dec 2021 23:58:16 +0300 Subject: [PATCH 4/4] add tests for format string compile-time checks --- test/compile-error-test/CMakeLists.txt | 32 ++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/compile-error-test/CMakeLists.txt b/test/compile-error-test/CMakeLists.txt index 5b0a172ffd6e..db7a94296048 100644 --- a/test/compile-error-test/CMakeLists.txt +++ b/test/compile-error-test/CMakeLists.txt @@ -167,5 +167,37 @@ expect_compile(udl-check " #endif ") +if (CMAKE_CXX_STANDARD GREATER_EQUAL 20) + # Compile-time argument type check + expect_compile(format-string-number-spec " + #ifdef FMT_HAS_CONSTEVAL + fmt::format(\"{:d}\", 42); + #endif + ") + expect_compile(format-string-number-spec-error " + #ifdef FMT_HAS_CONSTEVAL + fmt::format(\"{:d}\", \"I am not a number\"); + #else + #error + #endif + " ERROR) + + # Compile-time argument name check + expect_compile(format-string-name " + #if defined(FMT_HAS_CONSTEVAL) && FMT_USE_NONTYPE_TEMPLATE_PARAMETERS + using namespace fmt::literals; + fmt::print(\"{foo}\", \"foo\"_a=42); + #endif + ") + expect_compile(format-string-name-error " + #if defined(FMT_HAS_CONSTEVAL) && FMT_USE_NONTYPE_TEMPLATE_PARAMETERS + using namespace fmt::literals; + fmt::print(\"{foo}\", \"bar\"_a=42); + #else + #error + #endif + " ERROR) +endif () + # Run all tests run_tests()