From 6c63e7b6adf56d1bcf5eb8e6ddbcb724015a2f18 Mon Sep 17 00:00:00 2001 From: TheCoconutChef Date: Sat, 15 Oct 2022 22:37:22 -0400 Subject: [PATCH 1/3] improve compile time message on assignation failure Consider the following simple program: ``` int main() { lager::reader a; lager::reader b = a; return 0; } ``` Compiling it will produce about many lines of compiler messages, mostly relating to template deduction failure of a shared_ptr ctor. This noisy situation is improved by the use of sfinae statements in the reader to reader ctor and cursor to reader ctor. In this case, an attempt to compile the aforementioned program will fail with about 4 lines of compiler messages directly quoting the offending line. N.B. this commit does not cover the case: ``` lager::reader a = lager::with(...); ``` --- lager/cursor.hpp | 5 ++++- lager/reader.hpp | 10 ++++++++-- lager/watch.hpp | 4 +++- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lager/cursor.hpp b/lager/cursor.hpp index 0ca5779a..39b3f66a 100644 --- a/lager/cursor.hpp +++ b/lager/cursor.hpp @@ -55,7 +55,10 @@ class cursor_base cursor_base() = default; - template + template , + zug::meta::value_t>, + int> = 0> cursor_base(cursor_base x) : base_t{std::move(x)} {} diff --git a/lager/reader.hpp b/lager/reader.hpp index 56a4a2ac..2f4ec52d 100644 --- a/lager/reader.hpp +++ b/lager/reader.hpp @@ -92,12 +92,18 @@ class reader_base reader_base() = default; - template + template , + zug::meta::value_t>, + int> = 0> reader_base(reader_base x) : base_t{std::move(x)} {} - template + template , + zug::meta::value_t>, + int> = 0> reader_base(cursor_base x) : base_t{std::move(x)} {} diff --git a/lager/watch.hpp b/lager/watch.hpp index 001fe3b3..2a7641cb 100644 --- a/lager/watch.hpp +++ b/lager/watch.hpp @@ -44,7 +44,9 @@ class watchable_base : private NodeT::signal_type::forwarder_type : node_{std::move(p)} {} - template + template >, + int> = 0> watchable_base(watchable_base x) : node_(std::move(x.node_)) {} From ba9d8e49f89fd6c55c53c0bbfe44b9f2d1845b3d Mon Sep 17 00:00:00 2001 From: TheCoconutChef Date: Sat, 15 Oct 2022 23:19:55 -0400 Subject: [PATCH 2/3] with: add sfinae for nicer compile failure messages Had we not been operating in the context of a CRTP, the sfinae would have been: ``` template ().make())::value_type > , int> = 0 > ``` However, this cannot work, for then compilers will complain that we're referencing an incomplete type in the form of std::declval. We need to delay the type instantiation with the following form: ``` template ().make())::value_type > , int> = 0 > ``` As far as I can tell, it's essentially this problem: https://stackoverflow.com/questions/43307462 --- lager/with.hpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lager/with.hpp b/lager/with.hpp index bf84531f..c13166d8 100644 --- a/lager/with.hpp +++ b/lager/with.hpp @@ -172,7 +172,14 @@ class with_expr_base : public xform_mixin return deriv_().zoom(lens); } - template + template ().make())::value_type, + T + >, + int> = 0> operator reader() && { return std::move(*this).make(); @@ -182,7 +189,14 @@ class with_expr_base : public xform_mixin { return std::move(*this).make(); } - template + template ().make())::value_type, + T + >, + int> = 0> operator cursor() && { return std::move(*this).make(); From 9a56535c3b7db5dfa56ed7eb410ab6d703770f07 Mon Sep 17 00:00:00 2001 From: TheCoconutChef Date: Sun, 16 Oct 2022 08:20:03 -0400 Subject: [PATCH 3/3] test: add compile time test --- CMakeLists.txt | 1 + cmake/icm_build_failure_testing.cmake | 232 ++++++++++++++++++++++++ test/CMakeLists.txt | 7 +- test/build_failure/CMakeLists.txt | 19 ++ test/build_failure/cursor-to-cursor.cpp | 15 ++ test/build_failure/cursor-to-reader.cpp | 16 ++ test/build_failure/lense-to-cursor.cpp | 18 ++ test/build_failure/lense-to-reader.cpp | 17 ++ test/build_failure/map-to-reader.cpp | 15 ++ test/build_failure/reader-to-reader.cpp | 13 ++ test/build_failure/setup.hpp | 32 ++++ test/build_failure/with-to-reader.cpp | 17 ++ test/build_failure/xform-to-cursor.cpp | 18 ++ test/build_failure/xform-to-reader.cpp | 15 ++ 14 files changed, 434 insertions(+), 1 deletion(-) create mode 100644 cmake/icm_build_failure_testing.cmake create mode 100644 test/build_failure/CMakeLists.txt create mode 100644 test/build_failure/cursor-to-cursor.cpp create mode 100644 test/build_failure/cursor-to-reader.cpp create mode 100644 test/build_failure/lense-to-cursor.cpp create mode 100644 test/build_failure/lense-to-reader.cpp create mode 100644 test/build_failure/map-to-reader.cpp create mode 100644 test/build_failure/reader-to-reader.cpp create mode 100644 test/build_failure/setup.hpp create mode 100644 test/build_failure/with-to-reader.cpp create mode 100644 test/build_failure/xform-to-cursor.cpp create mode 100644 test/build_failure/xform-to-reader.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index f5fec1e0..3964dca3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20,6 +20,7 @@ if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") endif() option(lager_BUILD_TESTS "Build tests" ON) +option(lager_BUILD_FAILURE_TESTS "Build failure tests" ON) option(lager_BUILD_EXAMPLES "Build examples" ON) option(lager_BUILD_DEBUGGER_EXAMPLES "Build examples that showcase the web based debugger" ON) option(lager_BUILD_DOCS "Build docs" ON) diff --git a/cmake/icm_build_failure_testing.cmake b/cmake/icm_build_failure_testing.cmake new file mode 100644 index 00000000..9bdcff7d --- /dev/null +++ b/cmake/icm_build_failure_testing.cmake @@ -0,0 +1,232 @@ +# icm_build_failure_testing +# +# SPDX-License-Identifier: MIT +# MIT License: +# Copyright (c) 2022 Borislav Stanimirov +# +# Permission is hereby granted, free of charge, to any person obtaining +# a copy of this software and associated documentation files(the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and / or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions : +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +# NONINFRINGEMENT.IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE +# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION +# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +# +# VERSION HISTORY +# +# 1.00 (2022-xx-xx) Initial release +# +# NOTES +# This file is bundled with icm_build_failure_parse_and_run.cmake and expects +# it to be in the same directory +include_guard(GLOBAL) + +# store current dir to find icm_build_failure_parse_and_run +set(ICM_BUILD_FAILURE_TEST_SCRIPT_DIR "${CMAKE_CURRENT_LIST_DIR}") + +# icm_add_build_failure_test +# +# Add a build failure test to a project +# +# Args: +# * NAME name - Name of the test (for CTest) +# * TARGET target - Optional. +# Name of the target executable (whose build will fail). +# Defaults to '-test' if not provided +# +# * SOURCES sources - Sources for the executable +# * PARSE src - Optional. One of the sources can be marked as PARSE. If +# such is present, it will be parsed for expected errors +# +# * LIBRARIES libs - Optional. Link libraries for the executable +# * LABELS labels - Optional. CTest labels for the test +# If none are provided, "build-failure" will be added +# * FOLDER folder - Optional. MSVC solution folder for the target +# +# * ERROR_MATCHES mathes - Optional. Strings with which to match the build +# output of the executable +# +# Notes: +# * If neither ERROR_MATCHES nor a PARSE source is present, the test will only +# check that the build of the target fails. This is not recommended +# * If both are present, the PARSE source takes precedence +# * ERROR_MATCHES are a list of strings. The test will check that at least one +# of provided strings matches the output of the build +# * To list errors in the PARSE source, it must be on a line prepended with: +# "// build error:". Example: +# +# // x.cpp: +# // build error: custom error was triggered +# static_assert(false, "custom error was triggered") +# +# If multimple "// build error: lines" are present, at least one of them +# needs to match for a successful test. +# +# Example: +# icm_add_build_failure_test( +# NAME mylib-bf-foo-func-first-arg-trivial +# LIBRARIES mylib +# SOURCES +# test-helper.cpp +# PARSE bf-foo-func-first-arg-trivial.cpp +# LABELS bf mylib +# ) +function(icm_add_build_failure_test) + cmake_parse_arguments(ARG "" "NAME;TARGET;FOLDER" "SOURCES;LIBRARIES;ERROR_MATCHES;LABELS" ${ARGN}) + if(DEFINED ARG_UNPARSED_ARGUMENTS) + message(NOTICE "icm_add_build_failure_test called with unknown arguments") + endif() + + # check sources for a file to parse + cmake_parse_arguments(ARG "" "PARSE" "" ${ARG_SOURCES}) + set(ARG_SOURCES "${ARG_UNPARSED_ARGUMENTS}") + + if(NOT DEFINED ARG_TARGET) + set(ARG_TARGET "${ARG_NAME}-test") + endif() + + # add an executable + # an object library will be sufficient for compilation errors, + # but an executable will help with linker errors as well + add_executable(${ARG_TARGET} ${ARG_SOURCES}) + set_target_properties(${ARG_TARGET} PROPERTIES + EXCLUDE_FROM_ALL TRUE + EXCLUDE_FROM_DEFAULT_BUILD TRUE + ) + if(DEFINED ARG_LIBRARIES) + target_link_libraries(${ARG_TARGET} PRIVATE ${ARG_LIBRARIES}) + endif() + if(DEFINED ARG_FOLDER) + set_target_properties(${ARG_TARGET} PROPERTIES FOLDER ${ARG_FOLDER}) + endif() + + if(DEFINED ARG_PARSE) + # we find error matches from a parsed file + + # also add the parsed source to the executable's sources + target_sources(${ARG_TARGET} PRIVATE "${ARG_PARSE}") + # this var is used in the configured file bellow + get_filename_component(parsedSourcePath "${ARG_PARSE}" ABSOLUTE) + + # configure in binary dir based on target name + # it's safe as no two targets can have the same name anyway + # TODO: when minimal supported version of CMake is 3.18, embed the + # helper script [=[here]=] and use file(CONFIGURE) + configure_file( + "${ICM_BUILD_FAILURE_TEST_SCRIPT_DIR}/icm_build_failure_parse_and_run.cmake" + "${CMAKE_BINARY_DIR}/${ARG_TARGET}.cmake" + @ONLY + ) + + add_test( + NAME ${ARG_NAME} + # provide the config as a command line arg here + # we cannot configure the file with a generator expression + COMMAND ${CMAKE_COMMAND} -DCFG=$ -P ${ARG_TARGET}.cmake + WORKING_DIRECTORY ${CMAKE_BINARY_DIR} + ) + else() + # we look for error matches in arguments + add_test( + NAME ${ARG_NAME} + COMMAND ${CMAKE_COMMAND} --build . --target ${ARG_TARGET} --config $ + WORKING_DIRECTORY ${CMAKE_BINARY_DIR} + ) + if(DEFINED ARG_ERROR_MATCHES) + # matches are provided for all test runs + set_tests_properties(${ARG_NAME} PROPERTIES + PASS_REGULAR_EXPRESSION "${ARG_ERROR_MATCHES}") + else() + # no matches are provided so just expect the build to fail + set_tests_properties(${ARG_NAME} PROPERTIES + WILL_FAIL TRUE) + endif() + endif() + + if(NOT DEFINED ARG_LABELS) + # if labels are not provided, still add "build-failure" + set(ARG_LABELS "build-failure") + endif() + + set_tests_properties(${ARG_NAME} PROPERTIES LABELS "${ARG_LABELS}") +endfunction() + +# icm_add_multiple_build_failure_tests +# +# Add a multiple build failure tests to a project via multiple calls to +# icm_add_build_failure_test +# +# Args: +# * SOURCES sources - List of sources to add. Each source will lead to a new +# test being added +# * PREFIX prefix - Optional. Prefix string to add to each test name +# +# * LIBRARIES, LABELS, FOLDER, ERROR_MATCHES +# forwarded to icm_add_build_failure_test +# +# Notes: +# If ERROR_MATCHES is not present, each source is forwarded as PARSE +# +function(icm_add_multiple_build_failure_tests) + cmake_parse_arguments(ARG "" "PREFIX;FOLDER" "SOURCES;LIBRARIES;ERROR_MATCHES;LABELS" ${ARGN}) + if(DEFINED ARG_UNPARSED_ARGUMENTS) + message(NOTICE "icm_add_multiple_build_failure_tests called with unknown arguments") + endif() + foreach(sourceFile ${ARG_SOURCES}) + # replace spaces and path separators with '-' + string(REGEX REPLACE "[// ]" "-" testName "${sourceFile}") + # strip extenstion for target name + get_filename_component(testName ${testName} NAME_WLE) + + if(NOT DEFINED ARG_ERROR_MATCHES) + set(sourceFile "PARSE;${sourceFile}") + endif() + + if(DEFINED ARG_PREFIX) + set(testName ${ARG_PREFIX}-${testName}) + endif() + + # add test and forward args + icm_add_build_failure_test( + NAME ${testName} + SOURCES "${sourceFile}" + LIBRARIES ${ARG_LIBRARIES} + LABELS ${ARG_LABELS} + ERROR_MATCHES "${ARG_ERROR_MATCHES}" + FOLDER ${ARG_FOLDER} + ) + endforeach() +endfunction() + +# icm_glob_build_failure_tests +# +# add multiple sources to icm_add_multiple_build_failure_tests via a +# file(GLOB pattern) +# +# Args: +# * PATTERN pat - GLOB pattern to use +# The rest of the argument are forwarded verbatim to +# icm_add_multiple_build_failure_tests +function(icm_glob_build_failure_tests) + cmake_parse_arguments(ARG "" "PATTERN" "" ${ARGN}) + file(GLOB srcs + RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} + LIST_DIRECTORIES OFF + ${ARG_PATTERN} + ) + icm_add_multiple_build_failure_tests( + SOURCES ${srcs} + ${ARG_UNPARSED_ARGUMENTS} + ) +endfunction() diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 0c185466..2f164d50 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -49,7 +49,7 @@ find_package(Qt5 COMPONENTS Core Concurrent) add_custom_target(tests COMMENT "Build all the unit tests.") add_dependencies(check tests) -file(GLOB_RECURSE lager_unit_tests "*.cpp") +file(GLOB lager_unit_tests "*.cpp" "cereal/*.cpp" "event_loop/*.cpp" "extra/*.cpp") foreach(_file IN LISTS lager_unit_tests) message("found unit test: " ${_file}) @@ -66,3 +66,8 @@ foreach(_file IN LISTS lager_unit_tests) target_link_libraries(${_target} PUBLIC lager-dev ${_qt_libs}) add_test("test/${_output}" ${_output}) endforeach() + + +if (lager_BUILD_FAILURE_TESTS) + add_subdirectory(build_failure) +endif() diff --git a/test/build_failure/CMakeLists.txt b/test/build_failure/CMakeLists.txt new file mode 100644 index 00000000..9e28b612 --- /dev/null +++ b/test/build_failure/CMakeLists.txt @@ -0,0 +1,19 @@ +include(icm_build_failure_testing) + +set(_glob_pattern "*.cpp") + +file(GLOB_RECURSE lager_build_failure_tests ${_glob_pattern}) +foreach(_file IN LISTS lager_build_failure_tests) + message("found build failure test: " ${_file}) +endforeach() + +# We test that the build fails and the failure message +# contains the offending line. +# +# see: https://ibob.bg/blog/2022/10/04/testing-build-failure-with-cmake/ +icm_glob_build_failure_tests( + PATTERN ${_glob_pattern} + LIBRARIES lager-dev + PREFIX "build-failure" + ERROR_MATCHES "offending line" +) diff --git a/test/build_failure/cursor-to-cursor.cpp b/test/build_failure/cursor-to-cursor.cpp new file mode 100644 index 00000000..2e4a4de9 --- /dev/null +++ b/test/build_failure/cursor-to-cursor.cpp @@ -0,0 +1,15 @@ +// clang-format off +#include + +#include + +int main() +{ + lager::cursor a; + + lager::cursor int_cursor = a; // happy path + + lager::cursor b = a; // offending line + + return 0; +} diff --git a/test/build_failure/cursor-to-reader.cpp b/test/build_failure/cursor-to-reader.cpp new file mode 100644 index 00000000..c4d639ea --- /dev/null +++ b/test/build_failure/cursor-to-reader.cpp @@ -0,0 +1,16 @@ +// clang-format off +#include +#include + +#include + +int main() +{ + lager::cursor a{}; + + lager::reader int_reader = a; // happy path + + lager::reader b = a; // offending line + + return 0; +} diff --git a/test/build_failure/lense-to-cursor.cpp b/test/build_failure/lense-to-cursor.cpp new file mode 100644 index 00000000..57c0e565 --- /dev/null +++ b/test/build_failure/lense-to-cursor.cpp @@ -0,0 +1,18 @@ +// clang-format off +#include +#include +#include + +#include "setup.hpp" + +int main() +{ + auto st = lager::make_state(Model{}); + auto l = lager::lenses::attr(&Model::foo); + + lager::cursor foo_cursor = st.zoom(l); // happy path + + lager::cursor a = st.zoom(l); // offending line + + return 0; +} diff --git a/test/build_failure/lense-to-reader.cpp b/test/build_failure/lense-to-reader.cpp new file mode 100644 index 00000000..eab11ca1 --- /dev/null +++ b/test/build_failure/lense-to-reader.cpp @@ -0,0 +1,17 @@ +// clang-format off +#include "setup.hpp" + +#include +#include + +int main() +{ + auto s = make_dummy_store(); + auto l = lager::lenses::attr(&Model::foo); + + lager::reader foo_reader = s.zoom(l); // happy path + + lager::reader b = s.zoom(l); // offending line + + return 0; +} diff --git a/test/build_failure/map-to-reader.cpp b/test/build_failure/map-to-reader.cpp new file mode 100644 index 00000000..efcfa9ea --- /dev/null +++ b/test/build_failure/map-to-reader.cpp @@ -0,0 +1,15 @@ +// clang-format off +#include + +#include "setup.hpp" + +int main() +{ + auto s = make_dummy_store(); + + lager::reader int_reader = s.map([](auto) { return 1; }); // happy path + + lager::reader a = s.map([](auto) { return 1; }); // offending line + + return 0; +} diff --git a/test/build_failure/reader-to-reader.cpp b/test/build_failure/reader-to-reader.cpp new file mode 100644 index 00000000..4277d7b3 --- /dev/null +++ b/test/build_failure/reader-to-reader.cpp @@ -0,0 +1,13 @@ +// clang-format off +#include + +int main() +{ + lager::reader a{}; + + lager::reader num = a; // happy path + + lager::reader b = a; // offending line + + return 0; +} diff --git a/test/build_failure/setup.hpp b/test/build_failure/setup.hpp new file mode 100644 index 00000000..bd3defb0 --- /dev/null +++ b/test/build_failure/setup.hpp @@ -0,0 +1,32 @@ +#pragma once + +#include + +#include +#include + +#include +#include + +struct Foo +{ + int b{1}; + LAGER_STRUCT_NESTED(Foo, b); +}; +struct Model +{ + int a{0}; + Foo foo{}; + LAGER_STRUCT_NESTED(Model, a); +}; +struct ModelAction +{ + LAGER_STRUCT_NESTED(ModelAction); +}; + +using Action = std::variant; +Model update(Model m, Action a) { return m; } + +auto make_dummy_store() { + return lager::make_store(Model{}, lager::with_manual_event_loop{}); +} diff --git a/test/build_failure/with-to-reader.cpp b/test/build_failure/with-to-reader.cpp new file mode 100644 index 00000000..eb8e79dc --- /dev/null +++ b/test/build_failure/with-to-reader.cpp @@ -0,0 +1,17 @@ +// clang-format off +#include +#include + +#include "setup.hpp" + +int main() +{ + lager::reader a{}; + lager::reader c{}; + + lager::reader> with_reader = lager::with(a, c); // happy path + + lager::reader b = lager::with(a, c); // offending line + + return 0; +} diff --git a/test/build_failure/xform-to-cursor.cpp b/test/build_failure/xform-to-cursor.cpp new file mode 100644 index 00000000..c0987cd7 --- /dev/null +++ b/test/build_failure/xform-to-cursor.cpp @@ -0,0 +1,18 @@ +// clang-format off +#include + +#include + +struct Foo +{}; + +int main() +{ + lager::cursor str{}; + + lager::cursor num = str.xform(zug::map([](std::string) { return 1; }), zug::map([](int) { return ""; })); // happy path + + lager::cursor num_fail = str.xform(zug::map([](std::string) { return 1; }), zug::map([](int) { return ""; })); // offending line + + return 0; +} diff --git a/test/build_failure/xform-to-reader.cpp b/test/build_failure/xform-to-reader.cpp new file mode 100644 index 00000000..f7c9f59d --- /dev/null +++ b/test/build_failure/xform-to-reader.cpp @@ -0,0 +1,15 @@ +// clang-format off +#include + +struct Foo{}; + +int main() +{ + lager::reader num{}; + + lager::reader str = num.xform(zug::map([](int) { return std::string{""}; })); // happy path + + lager::reader fail = num.xform(zug::map([](int) { return ""; })); // offending line + + return 0; +}