CXX-2767 Add macro guards tests for bsoncxx and mongocxx #1043
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,107 @@ | ||||||||||||||||||||||||||
| # PROJECT_NAME: bsoncxx or mongocxx. | ||||||||||||||||||||||||||
| # GUARDED_MACROS: list of macros that should be guarded by prelude/postlude headers. | ||||||||||||||||||||||||||
| # INCLUDE_FILTERS: list of regex filters for headers to be added to the tests (must be relative to PROJECT_SOURCE_DIR). | ||||||||||||||||||||||||||
| # EXCLUDE_FILTERS: list of regex filters for headers to be excluded (applied after INCLUDE_FILTERS). | ||||||||||||||||||||||||||
| # PROJECT_TEST_PROPERTIES: the (bsoncxx|mongocxx)_test_properties target to be linked with. | ||||||||||||||||||||||||||
| function(add_macro_guard_test) | ||||||||||||||||||||||||||
| set(opt_args "") | ||||||||||||||||||||||||||
| set(single_args "PROJECT_NAME;PROJECT_TEST_PROPERTIES") | ||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Suggested change
or
Suggested change
The name There was a problem hiding this comment.
That's more or less the purpose that the |
||||||||||||||||||||||||||
| set(multi_args "GUARDED_MACROS;INCLUDE_FILTERS;EXCLUDE_FILTERS") | ||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Suggested change
Rename suggested since they accept different syntaxes |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| cmake_parse_arguments(PARSED "${opt_args}" "${single_args}" "${multi_args}" ${ARGN}) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if(NOT "${PARSED_UNPARSED_ARGUMENTS}" STREQUAL "") | ||||||||||||||||||||||||||
| message(FATAL_ERROR "unrecognized argument: ${PARSED_UNPARSED_ARGUMENTS}") | ||||||||||||||||||||||||||
| endif() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| foreach(required_arg PROJECT_NAME PROJECT_TEST_PROPERTIES GUARDED_MACROS INCLUDE_FILTERS) | ||||||||||||||||||||||||||
| if("${PARSED_${required_arg}}" STREQUAL "") | ||||||||||||||||||||||||||
| message(FATAL_ERROR "missing required argument: ${required_arg}") | ||||||||||||||||||||||||||
| endif() | ||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Suggested change
This permits empty strings as values, but that may be part of the check. Alternatively, tweak the error message:
Suggested change
There was a problem hiding this comment. Yes, an empty argument is considered invalid input for these parameters. Your suggestion pointed me to |
||||||||||||||||||||||||||
| endforeach() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| set(GUARDED_HEADERS "") | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| foreach(filter ${PARSED_INCLUDE_FILTERS}) | ||||||||||||||||||||||||||
| file(GLOB_RECURSE headers | ||||||||||||||||||||||||||
| LIST_DIRECTORIES false | ||||||||||||||||||||||||||
| RELATIVE ${PROJECT_SOURCE_DIR} | ||||||||||||||||||||||||||
| "${PROJECT_SOURCE_DIR}/${filter}" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| list(APPEND GUARDED_HEADERS ${headers}) | ||||||||||||||||||||||||||
| endforeach() | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. You can pass multiple glob patterns to
Suggested change
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| foreach(filter ${PARSED_EXCLUDE_FILTERS}) | ||||||||||||||||||||||||||
| list(FILTER GUARDED_HEADERS EXCLUDE REGEX "${filter}") | ||||||||||||||||||||||||||
| endforeach() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| set(MACRO_GUARD_TEST_PRELUDE "") | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Check and set initial state. | ||||||||||||||||||||||||||
| foreach(macro ${PARSED_GUARDED_MACROS}) | ||||||||||||||||||||||||||
| string(APPEND MACRO_GUARD_TEST_PRELUDE | ||||||||||||||||||||||||||
| "#if defined(${macro})\n" | ||||||||||||||||||||||||||
| "#error \"${macro} is already defined\"\n" | ||||||||||||||||||||||||||
| "#endif\n" | ||||||||||||||||||||||||||
| "#define ${macro} macro guard test\n" | ||||||||||||||||||||||||||
| "\n" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| endforeach() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Implement as recursive algorithm for C++11 compatibility. | ||||||||||||||||||||||||||
| string(APPEND MACRO_GUARD_TEST_PRELUDE | ||||||||||||||||||||||||||
| "static constexpr bool compare_equal(const char* lhs, const char* rhs) {\n" | ||||||||||||||||||||||||||
| " return (*lhs == *rhs) && (*lhs == '\\0' || compare_equal(lhs + 1, rhs + 1));\n" | ||||||||||||||||||||||||||
| "}\n" | ||||||||||||||||||||||||||
| "\n" | ||||||||||||||||||||||||||
| "static_assert(compare_equal(\"abc\", \"abc\"), \"compare_equal() sanity check failed\");\n" | ||||||||||||||||||||||||||
| "static_assert(!compare_equal(\"abc\", \"def\"), \"compare_equal() sanity check failed\");\n" | ||||||||||||||||||||||||||
| "\n" | ||||||||||||||||||||||||||
| "#define _TO_STR(x) #x\n" | ||||||||||||||||||||||||||
| "#define TO_STR(x) _TO_STR(x)\n" | ||||||||||||||||||||||||||
| "\n" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
|
Comment on lines
+75
to
+86
There was a problem hiding this comment. Use a There was a problem hiding this comment. I am preferential towards being consistent with the other quoted arguments that require variable expansions (incompatible with bracket arguments) rather than using a bracket argument that requires unusual indentation to avoid it being reflected in the generated source file: # Check and set initial state.
foreach(macro ${PARSED_GUARDED_MACROS})
string(APPEND MACRO_GUARD_TEST_PRELUDE
"#if defined(${macro})\n"
"#error \"${macro} is already defined\"\n"
"#endif\n"
"#define ${macro} macro guard test\n"
"\n"
)
endforeach()
# Implement as recursive algorithm for C++11 compatibility.
string(APPEND MACRO_GUARD_TEST_PRELUDE [[
static constexpr bool compare_equal(const char* lhs, const char* rhs) {
return (*lhs == *rhs) && (*lhs == '\0' || compare_equal(lhs + 1, rhs + 1));
}
static_assert(compare_equal("abc", "abc"), "compare_equal() sanity check failed");
static_assert(!compare_equal("abc", "def"), "compare_equal() sanity check failed");
#define _TO_STR(x) #x
#define TO_STR(x) _TO_STR(x)
]])Would you rather we still use a bracket argument here? There was a problem hiding this comment. I'll leave it up to you. In such cases I don't usually mind odd indentation in generated sources, and I use |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| add_custom_target(test_${PARSED_PROJECT_NAME}_macro_guards) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Test each header individually. | ||||||||||||||||||||||||||
| foreach(header ${GUARDED_HEADERS}) | ||||||||||||||||||||||||||
| set(MACRO_GUARD_TEST "${MACRO_GUARD_TEST_PRELUDE}") | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Strip the subdir. | ||||||||||||||||||||||||||
| string(REGEX REPLACE "^(include|lib|test)/(.*)$" "\\1" subdir "${header}") | ||||||||||||||||||||||||||
| string(REGEX REPLACE "^(include|lib|test)/(.*)$" "\\2" header ${header}) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Apply include prefix to test headers. | ||||||||||||||||||||||||||
| if("${subdir}" STREQUAL "test") | ||||||||||||||||||||||||||
| set(relheader "${PARSED_PROJECT_NAME}/test/${header}") | ||||||||||||||||||||||||||
| else() | ||||||||||||||||||||||||||
| set(relheader "${header}") | ||||||||||||||||||||||||||
| endif() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # The include directive. | ||||||||||||||||||||||||||
| string(APPEND MACRO_GUARD_TEST "#include <${relheader}>\n\n") | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Test all guarded macros have been properly restored. | ||||||||||||||||||||||||||
| foreach(macro ${PARSED_GUARDED_MACROS}) | ||||||||||||||||||||||||||
| string(APPEND MACRO_GUARD_TEST | ||||||||||||||||||||||||||
| "static_assert(\n" | ||||||||||||||||||||||||||
| " compare_equal(TO_STR(${macro}),\"macro guard test\"),\n" | ||||||||||||||||||||||||||
| " \"${macro} was not correctly restored by <${relheader}>\"\n" | ||||||||||||||||||||||||||
| ");\n" | ||||||||||||||||||||||||||
| "\n" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| endforeach() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # e.g. bsoncxx/v_noabi/bsoncxx/document/view.hpp -> bsoncxx-v_noabi-bsoncxx-document-view | ||||||||||||||||||||||||||
| string(REPLACE "/" "-" test_name "${header}") | ||||||||||||||||||||||||||
| string(REGEX REPLACE "^(.*)\\.(hh|hpp)$" "\\1" test_name "${test_name}") | ||||||||||||||||||||||||||
| configure_file(test_macro_guards.cpp.in macro_guards/${subdir}/${test_name}.cpp) | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Since the file only expands to a single string, can this be a There was a problem hiding this comment. I considered
There was a problem hiding this comment. I was unaware of this feature of
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Prefix library name to reduce potential for target conflicts. | ||||||||||||||||||||||||||
| add_library(test-macro_guards-${subdir}-${test_name} STATIC EXCLUDE_FROM_ALL ${CMAKE_CURRENT_BINARY_DIR}/macro_guards/${subdir}/${test_name}.cpp) | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Does this need an individual target for every header? Could it be a single target with multiple source files attached? There was a problem hiding this comment. Now that you mention it, that makes a lot of sense! 😅 Will do. |
||||||||||||||||||||||||||
| target_link_libraries(test-macro_guards-${subdir}-${test_name} PRIVATE ${PARSED_PROJECT_TEST_PROPERTIES}) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| add_dependencies(test_${PARSED_PROJECT_NAME}_macro_guards test-macro_guards-${subdir}-${test_name}) | ||||||||||||||||||||||||||
| endforeach() | ||||||||||||||||||||||||||
| endfunction() | ||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a bracket-comment for easier reading+writing: