CXX-2767 Add macro guards tests for bsoncxx and mongocxx (#1043)

* Use static library for catch/main.cpp

* Add include/ and lib/ to target include directories

* Add macro guard tests

* Fix issues preventing standalone header inclusion

* Fix missing prelude header include directives

* Add macro guards tests to EVG config

* Simplify and improve robustness of test library targets

* CXX-2769 Document out-of-place BSONCXX_ENUM guards in macro guard headers

* CXX-2770 Workaround missing postlude headers via a test macro

Author: eramongodb

.evergreen/compile.sh

@@ -51,6 +51,15 @@ esac
 
 cd build
 "${cmake_binary}" -G "$GENERATOR" "-DCMAKE_BUILD_TYPE=${BUILD_TYPE}" -DBUILD_TESTING=ON -DMONGOCXX_ENABLE_SLOW_TESTS=ON -DENABLE_UNINSTALL=ON "$@" ..
+
+if [[ "${COMPILE_MACRO_GUARD_TESTS:-"OFF"}" == "ON" ]]; then
+   # We only need to compile the macro guard tests.
+   "${cmake_binary}" -DENABLE_MACRO_GUARD_TESTS=ON ..
+   "${cmake_binary}" --build . --config $BUILD_TYPE --target test_bsoncxx_macro_guards test_mongocxx_macro_guards -- $CMAKE_BUILD_OPTS
+   exit # Nothing else to be done.
+fi
+
+# Regular build and install routine.
 "${cmake_binary}" --build . --config $BUILD_TYPE -- $CMAKE_BUILD_OPTS
 "${cmake_binary}" --build . --config $BUILD_TYPE --target install -- $CMAKE_BUILD_OPTS
 "${cmake_binary}" --build . --config $BUILD_TYPE --target $CMAKE_EXAMPLES_TARGET -- $CMAKE_BUILD_OPTS

.mci.yml

@@ -380,6 +380,7 @@ functions:
                   set -o errexit
                   set -o pipefail
                   export BUILD_TYPE=${build_type}
+                  export COMPILE_MACRO_GUARD_TESTS=${COMPILE_MACRO_GUARD_TESTS}
                   export PATH="${extra_path}:$PATH"
                   export RUN_DISTCHECK=${RUN_DISTCHECK}
 
@@ -957,6 +958,14 @@ tasks:
         vars:
             ENABLE_TESTS: OFF
 
+    - name: compile_macro_guard_tests
+      commands:
+      - func: "setup"
+      - func: "fetch_c_driver_source"
+      - func: "compile"
+        vars:
+            COMPILE_MACRO_GUARD_TESTS: ON
+
     - name: compile_and_test_auth_with_shared_libs
       commands:
         - func: "setup"
@@ -1806,6 +1815,7 @@ buildvariants:
       tasks:
         - name: clang-tidy
         - name: compile_without_tests
+        - name: compile_macro_guard_tests
         - name: test_atlas_task_group_search_indexes
 
     - name: ubuntu2204-debug-gcc
@@ -1820,6 +1830,7 @@ buildvariants:
         - ubuntu2204-small
       tasks:
         - name: compile_without_tests
+        - name: compile_macro_guard_tests
 
     - name: ubuntu2204-debug-clang
       display_name: "Ubuntu 22.04 Debug (Clang)"
@@ -1833,6 +1844,7 @@ buildvariants:
         - ubuntu2204-small
       tasks:
         - name: compile_without_tests
+        - name: compile_macro_guard_tests
 
     - name: mongohouse-ubuntu
       display_name: "Mongohouse Test"

CMakeLists.txt

@@ -351,6 +351,7 @@ set(THIRD_PARTY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/src/third_party)
 set(DATA_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/data)
 
 option(ENABLE_TESTS "Build MongoDB C++ Driver tests." ON)
+option(ENABLE_MACRO_GUARD_TESTS "Enable targets for macro guard compile tests." OFF)
 
 if(ENABLE_TESTS)
     enable_testing()

cmake/BsoncxxUtil.cmake

@@ -46,8 +46,11 @@ function(bsoncxx_add_library TARGET OUTPUT_NAME LINK_TYPE)
         ${TARGET}
         PUBLIC
         $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include/bsoncxx/v_noabi>
+        $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
         $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/lib/bsoncxx/v_noabi>
+        $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/lib>
         $<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/lib/bsoncxx/v_noabi>
+        $<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/lib>
     )
     target_compile_definitions(${TARGET} PRIVATE ${libbson_definitions})
 endfunction(bsoncxx_add_library)

cmake/CMakeLists.txt

@@ -16,6 +16,7 @@ add_subdirectory(make_dist)
 
 set(cmake_MODULES
     ParseVersion.cmake
+    MacroGuardTest.cmake
     BsoncxxUtil.cmake
     MongocxxUtil.cmake
 )

cmake/MacroGuardTest.cmake

@@ -0,0 +1,135 @@
+#[==[
+	Define a build-time static library target whose compilation asserts that
+    header files are properly guarding config macros using inclusion of
+    prelude/postlude headers.
+
+    Usage:
+	
+	    add_macro_guard_test(
+	        PROJECT_NAME <name>
+	        PROJECT_TEST_PROPERTIES_TARGET <target>
+	        INCLUDE_PATTERNS [pattern...]
+	        [EXCLUDE_REGEXES [pattern...]]
+	        GUARDED_MACROS [macro...]
+	    )
+	
+	PROJECT_NAME
+	    Either "bsoncxx" or "mongocxx".
+	
+	PROJECT_TEST_PROPERTIES_TARGET
+	    Either the bsoncxx_test_properties or mongocxx_test_properties target.
+	
+	GUARDED_MACROS
+	    List of macros that should be guarded by prelude/postlude headers.
+	
+	INCLUDE_PATTERNS
+	    List of regex filters for headers to be added to the tests. Must be
+        relative to PROJECT_SOURCE_DIR.
+	
+	EXCLUDE_REGEXES
+	    List of regex filters for headers to be excluded. Applied after
+        INCLUDE_PATTERNS.
+]==]
+function(add_macro_guard_test)
+    set(opt_args "")
+    set(single_args "PROJECT_NAME;PROJECT_TEST_PROPERTIES_TARGET")
+    set(multi_args "GUARDED_MACROS;INCLUDE_PATTERNS;EXCLUDE_REGEXES")
+
+    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_TARGET GUARDED_MACROS INCLUDE_PATTERNS)
+        if("${required_arg}" IN_LIST PARSED_KEYWORDS_MISSING_VALUES)
+            message(FATAL_ERROR "missing value for required argument ${required_arg}")
+        endif()
+    endforeach()
+
+    list(TRANSFORM PARSED_INCLUDE_PATTERNS PREPEND "${PROJECT_SOURCE_DIR}/")
+    file(GLOB_RECURSE GUARDED_HEADERS
+        LIST_DIRECTORIES false
+        RELATIVE ${PROJECT_SOURCE_DIR}
+        ${PARSED_INCLUDE_PATTERNS}
+    )
+
+    foreach(filter ${PARSED_EXCLUDE_REGEXES})
+        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"
+    )
+
+    add_library(test_${PARSED_PROJECT_NAME}_macro_guards STATIC EXCLUDE_FROM_ALL)
+    target_link_libraries(test_${PARSED_PROJECT_NAME}_macro_guards PRIVATE ${PARSED_PROJECT_TEST_PROPERTIES_TARGET})
+
+    # 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()
+
+        # CXX-2770: workaround missing postlude header includes.
+        string(TOUPPER "${PARSED_PROJECT_NAME}" project_name_upper)
+        string(APPEND MACRO_GUARD_TEST "#define ${project_name_upper}_TEST_MACRO_GUARDS_FIX_MISSING_POSTLUDE\n\n")
+
+        # 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.cpp
+        string(REPLACE "/" "-" test_name "${header}")
+        string(REGEX REPLACE "^(.*)\\.(hh|hpp)$" "\\1" test_name "${test_name}")
+
+        # e.g. macro_guards/(include|lib|test)/bsoncxx-v_noabi-bsoncxx-document-view.cpp
+        configure_file(test_macro_guards.cpp.in macro_guards/${subdir}/${test_name}.cpp)
+
+        target_sources(test_${PARSED_PROJECT_NAME}_macro_guards PRIVATE
+            ${CMAKE_CURRENT_BINARY_DIR}/macro_guards/${subdir}/${test_name}.cpp
+        )
+    endforeach()
+endfunction()

cmake/MongocxxUtil.cmake

@@ -34,8 +34,11 @@ function(mongocxx_add_library TARGET OUTPUT_NAME LINK_TYPE)
         ${TARGET}
         PUBLIC
         $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include/mongocxx/v_noabi>
+        $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
         $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/lib/mongocxx/v_noabi>
+        $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/lib>
         $<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/lib/mongocxx/v_noabi>
+        $<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/lib>
     )
     target_compile_definitions(${TARGET} PRIVATE ${libmongoc_definitions})
 endfunction(mongocxx_add_library)

src/bsoncxx/cmake/CMakeLists.txt

@@ -54,7 +54,9 @@ if(1)
         RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT runtime
         LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT runtime
         ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT dev
-        INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/bsoncxx/v_noabi
+        INCLUDES DESTINATION
+        ${CMAKE_INSTALL_INCLUDEDIR}/bsoncxx/v_noabi
+        ${CMAKE_INSTALL_INCLUDEDIR}
     )
 
     install(EXPORT bsoncxx_targets
@@ -76,6 +78,7 @@ endif()
 if(1)
     set(PACKAGE_INCLUDE_INSTALL_DIRS
         ${CMAKE_INSTALL_INCLUDEDIR}/bsoncxx/v_noabi
+        ${CMAKE_INSTALL_INCLUDEDIR}
         ${BSONCXX_POLY_MNMLSTC_DEPRECATED_INCLUDE_DIRS}
     )
     set(PACKAGE_LIBRARY_INSTALL_DIRS ${CMAKE_INSTALL_LIBDIR})

src/bsoncxx/cmake/libbsoncxx-static.pc.in

@@ -21,5 +21,5 @@ Description: The MongoDB C++11 BSON Library
 URL: http://github.com/mongodb/mongo-cxx-driver
 Version: @BSONCXX_VERSION@
 Requires: libbson-static-@LIBBSON_REQUIRED_ABI_VERSION@ >= @LIBBSON_REQUIRED_VERSION@
-Cflags: @BSONCXX_POLY_MNMLSTC_PKGCONFIG_STATIC_INCLUDE_DIRS@-I${includedir}/bsoncxx/v_noabi -DBSONCXX_STATIC
+Cflags: @BSONCXX_POLY_MNMLSTC_PKGCONFIG_STATIC_INCLUDE_DIRS@-I${includedir}/bsoncxx/v_noabi -I${includedir} -DBSONCXX_STATIC
 Libs: -L${libdir} -lbsoncxx-static

src/bsoncxx/cmake/libbsoncxx.pc.in

@@ -20,5 +20,5 @@ Name: libbsoncxx
 Description: The MongoDB C++11 BSON Library
 URL: http://github.com/mongodb/mongo-cxx-driver
 Version: @BSONCXX_VERSION@
-Cflags: @BSONCXX_POLY_MNMLSTC_PKGCONFIG_INCLUDE_DIRS@-I${includedir}/bsoncxx/v_noabi
+Cflags: @BSONCXX_POLY_MNMLSTC_PKGCONFIG_INCLUDE_DIRS@-I${includedir}/bsoncxx/v_noabi -I${includedir}
 Libs: -L${libdir} -lbsoncxx

src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/builder/list.hpp

@@ -186,3 +186,8 @@ class array : public list {
 }  // namespace builder
 }  // namespace v_noabi
 }  // namespace bsoncxx
+
+// CXX-2770: missing include of postlude header.
+#if defined(BSONCXX_TEST_MACRO_GUARDS_FIX_MISSING_POSTLUDE)
+#include <bsoncxx/config/postlude.hpp>
+#endif

src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/config/postlude.hpp

@@ -45,12 +45,6 @@
 #undef BSONCXX_VERSION_PATCH
 #pragma pop_macro("BSONCXX_VERSION_PATCH")
 
-// bsoncxx/types.hpp
-#ifdef BSONCXX_ENUM
-static_assert(false, "BSONCXX_ENUM must be undef'ed");
-#endif
-#pragma pop_macro("BSONCXX_ENUM")
-
 // export.hpp (generated by CMake)
 #undef BSONCXX_API_H
 #pragma pop_macro("BSONCXX_API_H")
@@ -72,3 +66,9 @@ static_assert(false, "BSONCXX_ENUM must be undef'ed");
 // prelude.hpp
 #undef BSONCXX_UNREACHABLE
 #pragma pop_macro("BSONCXX_UNREACHABLE")
+
+// CXX-2769: out-of-place, but remains for backward compatibility.
+#ifdef BSONCXX_ENUM
+static_assert(false, "BSONCXX_ENUM must be undef'ed");
+#endif
+#pragma pop_macro("BSONCXX_ENUM")

src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/config/prelude.hpp

@@ -42,10 +42,6 @@
 #pragma push_macro("BSONCXX_VERSION_PATCH")
 #undef BSONCXX_VERSION_PATCH
 
-// bsoncxx/types.hpp
-#pragma push_macro("BSONCXX_ENUM")
-#undef BSONCXX_ENUM
-
 // export.hpp (generated by CMake)
 #pragma push_macro("BSONCXX_API_H")
 #undef BSONCXX_API_H
@@ -73,6 +69,10 @@
 #undef BSONCXX_UNREACHABLE
 #define BSONCXX_UNREACHABLE std::abort()
 
+// CXX-2769: out-of-place, but remains for backward compatibility.
+#pragma push_macro("BSONCXX_ENUM")
+#undef BSONCXX_ENUM
+
 // Doxygen does not account for generated header files.
 // Document globally applicable macros and namespaces here.
 

src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/types.hpp

@@ -13,13 +13,6 @@
 // limitations under the License.
 
 #pragma once
-#if defined(__clang__)
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wfloat-equal"
-#elif defined(__GNUC__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wfloat-equal"
-#endif
 
 #include <chrono>
 #include <cstring>
@@ -32,6 +25,17 @@
 
 #include <bsoncxx/config/prelude.hpp>
 
+#pragma push_macro("BSONCXX_ENUM")
+#undef BSONCXX_ENUM
+
+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wfloat-equal"
+#elif defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wfloat-equal"
+#endif
+
 namespace bsoncxx {
 inline namespace v_noabi {
 ///
@@ -683,10 +687,15 @@ BSONCXX_INLINE bool operator==(const b_maxkey&, const b_maxkey&) {
 }  // namespace v_noabi
 }  // namespace bsoncxx
 
-#include <bsoncxx/config/postlude.hpp>
-
 #if defined(__clang__)
 #pragma clang diagnostic pop
 #elif defined(__GNUC__)
 #pragma GCC diagnostic pop
 #endif
+
+#ifdef BSONCXX_ENUM
+static_assert(false, "BSONCXX_ENUM must be undef'ed");
+#endif
+#pragma pop_macro("BSONCXX_ENUM")
+
+#include <bsoncxx/config/postlude.hpp>

src/bsoncxx/lib/bsoncxx/v_noabi/bsoncxx/types/private/convert.hh

@@ -330,3 +330,5 @@ BSONCXX_INLINE void convert_from_libbson(bson_value_t* v, bsoncxx::types::b_arra
 }  // namespace types
 }  // namespace v_noabi
 }  // namespace bsoncxx
+
+#include <bsoncxx/config/private/postlude.hh>