From 9d4b4cf408d4fb34c9b2c0aeb772cf01bfd0a037 Mon Sep 17 00:00:00 2001 From: Eric Hughes Date: Tue, 17 Oct 2023 08:08:04 -0600 Subject: [PATCH] Add an aspect template argument to `class CAPIFunction`. This PR makes two changes to the template signature of `class CAPIFunction`. The central one is to add an aspect argument that can provide hooks into `CAPIFunction::function`. In this initial version there's a single aspect point (the aspect member function `call`) that is only an observer. The `call` of the default aspect does nothing. The second change in template signature is to infer the return value of the wrapper function and to suppress return statements if that type is `void`. This allows the elimination of subsidiary wrapper for `void` functions to give them a (constant) return value. Tests for the aspect implement `class LoggingAspect`, a primitive call-logger sufficient for testing, and provide an override header to activate it. The override header also generates a traits class for each C API implementation function that provides data for `LoggingAspect`. These elements are exercised by two new test runners that share a common set of tests; only the results differ depending on whether the hook is active or not. In order to automatically generate metadata for API functions, the PR introduces a set of macros to define the C API interface functions. These functions are already essentially boilerplate, but they cannot be defined with function templates because they must have "C" linkage. The macros provide a pair of macro hooks for generating code either before or after the function definition. --- tiledb/api/c_api/config/config_api.cc | 127 +++++---------- tiledb/api/c_api/context/context_api.cc | 90 +++++------ tiledb/api/c_api/error/error_api.cc | 12 +- tiledb/api/c_api_support/c_api_support.h | 4 + .../exception_wrapper/CMakeLists.txt | 1 - .../exception_wrapper/capi_definition.h | 114 +++++++++++++ .../exception_wrapper/exception_wrapper.h | 108 +++++++------ .../exception_wrapper/test/CMakeLists.txt | 50 ++++++ .../exception_wrapper/test/hook/DIRECTORY.md | 5 + .../test/hook/capi_function_override.h | 51 ++++++ .../exception_wrapper/test/hook_common.h | 45 ++++++ .../exception_wrapper/test/logging_aspect.h | 80 +++++++++ .../exception_wrapper/test/unit_capi_hook.cc | 152 ++++++++++++++++++ .../test/unit_capi_hook_with.cc | 37 +++++ .../test/unit_capi_hook_without.cc | 37 +++++ 15 files changed, 716 insertions(+), 197 deletions(-) create mode 100644 tiledb/api/c_api_support/exception_wrapper/capi_definition.h create mode 100644 tiledb/api/c_api_support/exception_wrapper/test/hook/DIRECTORY.md create mode 100644 tiledb/api/c_api_support/exception_wrapper/test/hook/capi_function_override.h create mode 100644 tiledb/api/c_api_support/exception_wrapper/test/hook_common.h create mode 100644 tiledb/api/c_api_support/exception_wrapper/test/logging_aspect.h create mode 100644 tiledb/api/c_api_support/exception_wrapper/test/unit_capi_hook.cc create mode 100644 tiledb/api/c_api_support/exception_wrapper/test/unit_capi_hook_with.cc create mode 100644 tiledb/api/c_api_support/exception_wrapper/test/unit_capi_hook_without.cc diff --git a/tiledb/api/c_api/config/config_api.cc b/tiledb/api/c_api/config/config_api.cc index fdcc8b1fa35..129d9008d45 100644 --- a/tiledb/api/c_api/config/config_api.cc +++ b/tiledb/api/c_api/config/config_api.cc @@ -5,7 +5,7 @@ * * The MIT License * - * @copyright Copyright (c) 2022 TileDB, Inc. + * @copyright Copyright (c) 2022-2023 TileDB, Inc. * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -172,122 +172,77 @@ capi_return_t tiledb_config_iter_done( } // namespace tiledb::api -using tiledb::api::api_entry_error; - -capi_return_t tiledb_config_alloc( - tiledb_config_t** config, tiledb_error_t** error) noexcept { - return api_entry_error(error, config); -} +CAPI_ERROR_BEGIN(config_alloc, tiledb_config_t** config) +CAPI_ERROR_END(config_alloc, config) /* * API Audit: Void return means no possible signal for an error. No channel that * can return an error. * Possible errors: `config` may be null or an invalid handle. */ -void tiledb_config_free(tiledb_config_t** config) noexcept { - return tiledb::api::api_entry_void(config); -} +CAPI_VOID_BEGIN(config_free, tiledb_config_t** config) +CAPI_VOID_END(config_free, config) -capi_return_t tiledb_config_set( - tiledb_config_t* config, - const char* param, - const char* value, - tiledb_error_t** error) noexcept { - return api_entry_error( - error, config, param, value); -} +CAPI_ERROR_BEGIN( + config_set, tiledb_config_t* config, const char* param, const char* value) +CAPI_ERROR_END(config_set, config, param, value) -capi_return_t tiledb_config_get( - tiledb_config_t* config, - const char* param, - const char** value, - tiledb_error_t** error) noexcept { - return api_entry_error( - error, config, param, value); -} +CAPI_ERROR_BEGIN( + config_get, tiledb_config_t* config, const char* param, const char** value) +CAPI_ERROR_END(config_get, config, param, value) -capi_return_t tiledb_config_unset( - tiledb_config_t* config, - const char* param, - tiledb_error_t** error) noexcept { - return api_entry_error( - error, config, param); -} +CAPI_ERROR_BEGIN(config_unset, tiledb_config_t* config, const char* param) +CAPI_ERROR_END(config_unset, config, param) -capi_return_t tiledb_config_load_from_file( - tiledb_config_t* config, - const char* filename, - tiledb_error_t** error) noexcept { - return api_entry_error( - error, config, filename); -} +CAPI_ERROR_BEGIN( + config_load_from_file, tiledb_config_t* config, const char* filename) +CAPI_ERROR_END(config_load_from_file, config, filename) -capi_return_t tiledb_config_save_to_file( - tiledb_config_t* config, - const char* filename, - tiledb_error_t** error) noexcept { - return api_entry_error( - error, config, filename); -} +CAPI_ERROR_BEGIN( + config_save_to_file, tiledb_config_t* config, const char* filename) +CAPI_ERROR_END(config_save_to_file, config, filename) /* * API Audit: No channel that can return an error. * Possible errors: Both `lhs` and `rhs` may be null or an invalid handle. * `equal` may be a null pointer */ -capi_return_t tiledb_config_compare( - tiledb_config_t* lhs, tiledb_config_t* rhs, uint8_t* equal) noexcept { - return tiledb::api::api_entry_plain( - lhs, rhs, equal); -} +CAPI_PLAIN_BEGIN( + config_compare, tiledb_config_t* lhs, tiledb_config_t* rhs, uint8_t* equal) +CAPI_PLAIN_END(config_compare, lhs, rhs, equal) -capi_return_t tiledb_config_iter_alloc( +CAPI_ERROR_BEGIN( + config_iter_alloc, tiledb_config_t* config, const char* prefix, - tiledb_config_iter_t** config_iter, - tiledb_error_t** error) noexcept { - return api_entry_error( - error, config, prefix, config_iter); -} + tiledb_config_iter_t** config_iter) +CAPI_ERROR_END(config_iter_alloc, config, prefix, config_iter) -capi_return_t tiledb_config_iter_reset( +CAPI_ERROR_BEGIN( + config_iter_reset, tiledb_config_t* config, tiledb_config_iter_t* config_iter, - const char* prefix, - tiledb_error_t** error) noexcept { - return api_entry_error( - error, config, config_iter, prefix); -} + const char* prefix) +CAPI_ERROR_END(config_iter_reset, config, config_iter, prefix) /* * API Audit: Void return means no possible signal for an error. No channel that * can return an error. * Possible errors: `config` may be null or an invalid handle. */ -void tiledb_config_iter_free(tiledb_config_iter_t** config_iter) noexcept { - tiledb::api::api_entry_void( - config_iter); -} +CAPI_VOID_BEGIN(config_iter_free, tiledb_config_iter_t** config_iter) +CAPI_VOID_END(config_iter_free, config_iter) -capi_return_t tiledb_config_iter_here( +CAPI_ERROR_BEGIN( + config_iter_here, tiledb_config_iter_t* config_iter, const char** param, - const char** value, - tiledb_error_t** error) noexcept { - return api_entry_error( - error, config_iter, param, value); -} + const char** value) +CAPI_ERROR_END(config_iter_here, config_iter, param, value) -capi_return_t tiledb_config_iter_next( - tiledb_config_iter_t* config_iter, tiledb_error_t** error) noexcept { - return api_entry_error( - error, config_iter); -} +CAPI_ERROR_BEGIN(config_iter_next, tiledb_config_iter_t* config_iter) +CAPI_ERROR_END(config_iter_next, config_iter) -capi_return_t tiledb_config_iter_done( - tiledb_config_iter_t* config_iter, - int32_t* done, - tiledb_error_t** error) noexcept { - return api_entry_error( - error, config_iter, done); -} \ No newline at end of file +CAPI_ERROR_BEGIN( + config_iter_done, tiledb_config_iter_t* config_iter, int32_t* done) +CAPI_ERROR_END(config_iter_done, config_iter, done) diff --git a/tiledb/api/c_api/context/context_api.cc b/tiledb/api/c_api/context/context_api.cc index 3b0eb566fbe..ae576c826c0 100644 --- a/tiledb/api/c_api/context/context_api.cc +++ b/tiledb/api/c_api/context/context_api.cc @@ -5,7 +5,7 @@ * * The MIT License * - * @copyright Copyright (c) 2022 TileDB, Inc. + * @copyright Copyright (c) 2022-2023 TileDB, Inc. * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -138,75 +138,67 @@ using tiledb::api::api_entry_with_context; /* * API Audit: No channel to return error message (failure code only) */ -capi_return_t tiledb_ctx_alloc( - tiledb_config_handle_t* config, tiledb_ctx_handle_t** ctx) noexcept { - return tiledb::api::api_entry_plain( - config, ctx); -} +CAPI_PLAIN_BEGIN( + ctx_alloc, tiledb_config_handle_t* config, tiledb_ctx_handle_t** ctx) +CAPI_PLAIN_END(ctx_alloc,config, ctx) /* * We have a special case with tiledb_ctx_alloc_with_error. It's declared in * tiledb_experimental.h. Rather than all the apparatus needed to split up that - * header (as tiledb.h is), we declare its linkage separately at the point of - * definition. + * header (as tiledb.h is), we declare it separately here, at the point of + * definition, with its correct link specification. * * Not including the experimental header means that we're not using the compiler * to check the definition against the declaration. This won't scale * particularly well, but it doesn't need to for the time being. */ extern "C" { - capi_return_t tiledb_ctx_alloc_with_error( tiledb_config_handle_t* config, tiledb_ctx_handle_t** ctx, - tiledb_error_handle_t** error) noexcept { - /* - * Wrapped with the `api_entry_error` variation. Note that the same function - * is wrapped with `api_entry_plain` above. - */ - return tiledb::api::api_entry_error( - error, config, ctx); -} - + tiledb_error_t** error) noexcept; } // extern "C" +/* + * This is a special case. The API function does not match the name of the + * implementation function. See`capi_definition.h` for more information. + */ +/* clang-format off */ +capi_return_t tiledb_ctx_alloc_with_error( + tiledb_config_handle_t* config, + tiledb_ctx_handle_t** ctx, + tiledb_error_t** error) +CAPI_MIDDLE(ctx_alloc, error) +CAPI_ERROR_END(ctx_alloc_with_error, config, ctx) +/* clang-format on */ + /* * API Audit: void return */ -void tiledb_ctx_free(tiledb_ctx_handle_t** ctx) noexcept { - return tiledb::api::api_entry_void(ctx); -} +CAPI_VOID_BEGIN(ctx_free, tiledb_ctx_handle_t** ctx) +CAPI_VOID_END(ctx_free,ctx) -capi_return_t tiledb_ctx_get_stats( - tiledb_ctx_t* ctx, char** stats_json) noexcept { - return api_entry_with_context( - ctx, stats_json); -} +CAPI_WITH_CONTEXT_BEGIN(ctx_get_stats, tiledb_ctx_t* ctx, char** stats_json) +CAPI_WITH_CONTEXT_END(ctx_get_stats,ctx, stats_json) -capi_return_t tiledb_ctx_get_config( - tiledb_ctx_t* ctx, tiledb_config_handle_t** config) noexcept { - return api_entry_with_context( - ctx, config); -} +CAPI_WITH_CONTEXT_BEGIN( + ctx_get_config, tiledb_ctx_t* ctx, tiledb_config_handle_t** config) +CAPI_WITH_CONTEXT_END(ctx_get_config, ctx, config) -capi_return_t tiledb_ctx_get_last_error( - tiledb_ctx_t* ctx, tiledb_error_handle_t** err) noexcept { - return api_entry_with_context( - ctx, err); -} +CAPI_WITH_CONTEXT_BEGIN( + ctx_get_last_error, tiledb_ctx_t* ctx, tiledb_error_handle_t** err) +CAPI_WITH_CONTEXT_END(ctx_get_last_error,ctx, err) -capi_return_t tiledb_ctx_is_supported_fs( - tiledb_ctx_t* ctx, tiledb_filesystem_t fs, int32_t* is_supported) noexcept { - return api_entry_with_context( - ctx, fs, is_supported); -} +CAPI_WITH_CONTEXT_BEGIN( + ctx_is_supported_fs, + tiledb_ctx_t* ctx, + tiledb_filesystem_t fs, + int32_t* is_supported) +CAPI_WITH_CONTEXT_END(ctx_is_supported_fs,ctx, fs, is_supported) -capi_return_t tiledb_ctx_cancel_tasks(tiledb_ctx_t* ctx) noexcept { - return api_entry_with_context(ctx); -} +CAPI_WITH_CONTEXT_BEGIN(ctx_cancel_tasks, tiledb_ctx_t* ctx) +CAPI_WITH_CONTEXT_END(ctx_cancel_tasks,ctx) -capi_return_t tiledb_ctx_set_tag( - tiledb_ctx_t* ctx, const char* key, const char* value) noexcept { - return api_entry_with_context( - ctx, key, value); -} +CAPI_WITH_CONTEXT_BEGIN( + ctx_set_tag, tiledb_ctx_t* ctx, const char* key, const char* value) +CAPI_WITH_CONTEXT_END(ctx_set_tag,ctx, key, value) diff --git a/tiledb/api/c_api/error/error_api.cc b/tiledb/api/c_api/error/error_api.cc index 88f6e3e5f95..0458b16001c 100644 --- a/tiledb/api/c_api/error/error_api.cc +++ b/tiledb/api/c_api/error/error_api.cc @@ -62,12 +62,8 @@ void tiledb_error_free(tiledb_error_handle_t** err) { } // namespace tiledb::api -capi_return_t tiledb_error_message( - tiledb_error_handle_t* err, const char** errmsg) noexcept { - return tiledb::api::api_entry_plain( - err, errmsg); -} +CAPI_PLAIN_BEGIN(error_message, tiledb_error_handle_t* err, const char** errmsg) +CAPI_PLAIN_END(error_message, err, errmsg) -void tiledb_error_free(tiledb_error_handle_t** err) noexcept { - return tiledb::api::api_entry_void(err); -} +CAPI_VOID_BEGIN(error_free, tiledb_error_handle_t** err) +CAPI_VOID_END(error_free, err) \ No newline at end of file diff --git a/tiledb/api/c_api_support/c_api_support.h b/tiledb/api/c_api_support/c_api_support.h index 90e736a745b..4ab9dbeb6e0 100644 --- a/tiledb/api/c_api_support/c_api_support.h +++ b/tiledb/api/c_api_support/c_api_support.h @@ -37,6 +37,10 @@ #define TILEDB_CAPI_SUPPORT_H #include "argument_validation.h" +#include "tiledb/api/c_api_support/exception_wrapper/capi_definition.h" #include "tiledb/api/c_api_support/exception_wrapper/exception_wrapper.h" +#if __has_include("capi_function_override.h") +#include "capi_function_override.h" +#endif #endif // TILEDB_CAPI_SUPPORT_H \ No newline at end of file diff --git a/tiledb/api/c_api_support/exception_wrapper/CMakeLists.txt b/tiledb/api/c_api_support/exception_wrapper/CMakeLists.txt index 2c658392b10..7d6669f31a5 100644 --- a/tiledb/api/c_api_support/exception_wrapper/CMakeLists.txt +++ b/tiledb/api/c_api_support/exception_wrapper/CMakeLists.txt @@ -33,7 +33,6 @@ include(object_library) # only. `exception_wrapper.cc` is an empty source file needed to allow the # OBJECT syntax. -# No actual source files at present. list(APPEND SOURCES exception_wrapper.cc ) diff --git a/tiledb/api/c_api_support/exception_wrapper/capi_definition.h b/tiledb/api/c_api_support/exception_wrapper/capi_definition.h new file mode 100644 index 00000000000..eece320e73e --- /dev/null +++ b/tiledb/api/c_api_support/exception_wrapper/capi_definition.h @@ -0,0 +1,114 @@ +/** + * @file tiledb/api/c_api_support/exception_wrapper/hook.h + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2023 TileDB, Inc. + * + * 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. + * + * @section DESCRIPTION + * + * This file defines macros to define C API functions. + */ + +#ifndef TILEDB_CAPI_DEFINITION_H +#define TILEDB_CAPI_DEFINITION_H + +/* + * The macro wrappers all have the same form + * ``` + * CAPI_XXX_BEGIN(name, ...) + * CAPI_XXX_END(...) + * ``` + * - The name argument is for the base name, that is, the name without the + * `tiledb_` prefix. + * - The `__VA_ARGS__` argument in `*_BEGIN` is the full argument part of the + * function signature. By "full", that means it requires that parameter names + * be present. + * - The `__VA_ARGS__` argument in `*_END` are the call arguments, that is, it's + * the function signature arguments without their type declarations. + */ + +/* clang-format off */ + +#define CAPI_DEFN(name) tiledb_##name +#define CAPI_IMPL(name) tiledb::api::tiledb_##name +#define CAPI_XFMR(name) tiledb::api::api_entry_##name +#define CAPI_PREFIX(name) +#define CAPI_SUFFIX(name) + +#define CAPI_BEGIN(root, ret) \ + CAPI_PREFIX(root) \ + ret CAPI_DEFN(root) + +#define CAPI_MIDDLE(root, xfmr) \ + noexcept { \ + return CAPI_XFMR(xfmr) + +#define CAPI_END(root, ...) \ + (__VA_ARGS__); \ + } \ + CAPI_SUFFIX(root) + +#define CAPI_PLAIN_BEGIN(root, ...) \ + CAPI_BEGIN(root, capi_return_t )(__VA_ARGS__) \ + CAPI_MIDDLE(root, plain) +#define CAPI_PLAIN_END(root, ...) \ + CAPI_END(root, __VA_ARGS__) + +#define CAPI_VOID_BEGIN(root, ...) \ + CAPI_BEGIN(root, void )(__VA_ARGS__) \ + CAPI_MIDDLE(root, void) +#define CAPI_VOID_END(root, ...) \ + CAPI_END(root, __VA_ARGS__) + +#define CAPI_WITH_CONTEXT_BEGIN(root, ...) \ + CAPI_BEGIN(root, capi_return_t)(__VA_ARGS__) \ + CAPI_MIDDLE(root, with_context) +#define CAPI_WITH_CONTEXT_END(root, ...) \ + CAPI_END(root, __VA_ARGS__) + +#define CAPI_CONTEXT_BEGIN(root, ...) \ + CAPI_BEGIN(root, capi_return_t)(__VA_ARGS__) \ + CAPI_MIDDLE(root, context) +#define CAPI_CONTEXT_END(root ,...) \ + CAPI_END(root, __VA_ARGS__) + +/* + * The argument lists for the ERROR macros omit the `error` argument. + * + * The ERROR wrapper is different from the others because it makes a special + * case of the `error` argument. Its definition is at the end of the argument + * list of the signature but at the beginning of the argument list of the call. + * + * Note that these macros, as written, do not handle zero-length variable + * arguments. There are no cases where this is needed. + */ +#define CAPI_ERROR_BEGIN(root, ...) \ + CAPI_BEGIN(root, capi_return_t)(__VA_ARGS__, tiledb_error_t * *error) \ + CAPI_MIDDLE(root, error) +#define CAPI_ERROR_END(root, ...) \ + CAPI_END(root, error, __VA_ARGS__) + +/* clang-format on */ + +#endif // TILEDB_CAPI_DEFINITION_H diff --git a/tiledb/api/c_api_support/exception_wrapper/exception_wrapper.h b/tiledb/api/c_api_support/exception_wrapper/exception_wrapper.h index 3a29e6401b4..709e2fd818f 100644 --- a/tiledb/api/c_api_support/exception_wrapper/exception_wrapper.h +++ b/tiledb/api/c_api_support/exception_wrapper/exception_wrapper.h @@ -514,18 +514,39 @@ using ExceptionActionCtxErr = detail::ExceptionActionDetailCtxErr; //------------------------------------------------------- // Exception wrapper //------------------------------------------------------- +template +class CAPIFunctionNullAspect { + public: + template + static void call(Args...) { + } +}; + +/** + * Selection struct defines the default aspect type for CAPIFunction. This class + * is always used with second template argument as `void`. This definition is + * for the general case; a specialization can override it. + */ +template +struct CAPIFunctionSelector { + using aspect_type = CAPIFunctionNullAspect; +}; + /** * Non-specialized wrapper for implementations functions for the C API. May * only be used as a specialization. */ -template +template < + auto f, + class H, + class A = typename CAPIFunctionSelector::aspect_type> class CAPIFunction; /** * Wrapper for implementations functions for the C API */ -template -class CAPIFunction { +template +class CAPIFunction { public: /** * Forwarded alias to template parameter H. @@ -539,7 +560,7 @@ class CAPIFunction { * @param args Arguments to an API implementation function * @return */ - static capi_return_t function(H& h, Args... args) { + static R function(H& h, Args... args) { /* * The order of the catch blocks is not arbitrary: * - `std::bad_alloc` comes first because it overrides other problems @@ -561,27 +582,46 @@ class CAPIFunction { * Note that we don't need std::forward here because all the arguments * must have "C" linkage. */ - auto x{f(args...)}; - h.action_on_success(); - return x; + //------------------------------------------------------- + A::call(args...); + if constexpr (std::same_as) { + f(args...); + h.action_on_success(); + } else { + auto x{f(args...)}; + h.action_on_success(); + return x; + } } catch (const std::bad_alloc& e) { h.action(e); - return TILEDB_OOM; + if constexpr (!std::same_as) { + return TILEDB_OOM; + } } catch (const detail::InvalidContextException& e) { h.action(e); - return TILEDB_INVALID_CONTEXT; + if constexpr (!std::same_as) { + return TILEDB_INVALID_CONTEXT; + } } catch (const detail::InvalidErrorException& e) { h.action(e); - return TILEDB_INVALID_ERROR; + if constexpr (!std::same_as) { + return TILEDB_INVALID_ERROR; + } } catch (const StatusException& e) { h.action(e); - return TILEDB_ERR; + if constexpr (!std::same_as) { + return TILEDB_ERR; + } } catch (const std::exception& e) { h.action(e); - return TILEDB_ERR; + if constexpr (!std::same_as) { + return TILEDB_ERR; + } } catch (...) { h.action(CAPIException("unknown exception type; no further information")); - return TILEDB_ERR; + if constexpr (!std::same_as) { + return TILEDB_ERR; + } } }; @@ -668,43 +708,6 @@ template constexpr auto api_entry_plain = CAPIFunction::function_plain; -/** - * Declaration only defined through a specialization. - * - * @tparam f An API implementation function - */ -template -struct CAPIFunctionVoid; - -/** - * Wrapper class for API implementation functions with `void` return. - * - * We require a separate wrapper class here so we can match the template - * argument `f` to a function with void return, since `CAPIFunction` only - * matches those that return `capi_return_t`. - * - * @tparam Args Argument types for the function - * @tparam f An API implementation function - */ -template -struct CAPIFunctionVoid { - /** - * Function transformer changes an API implementation function with `void` - * return to one returning a (trivially constant) `capi_return_t` value. - * - * This function is used to match the function signature in `CAPIFunction`, - * which requires a return value. This allows us to reuse its wrapper function - * without duplicating code. - * - * @param args Arguments passed to the function - * @return TILEDB_OK - */ - inline static capi_return_t function_from_void(Args... args) { - f(args...); - return TILEDB_OK; - } -}; - /** * Function transformer changes an API implementation function with `void` * return to an API interface function, also with `void` return. @@ -712,9 +715,8 @@ struct CAPIFunctionVoid { * @tparam f An implementation function. */ template -constexpr auto api_entry_void = CAPIFunction< - CAPIFunctionVoid::function_from_void, - tiledb::api::ExceptionAction>::void_function; +constexpr auto api_entry_void = + CAPIFunction::void_function; /** * Declaration only defined through a specialization. diff --git a/tiledb/api/c_api_support/exception_wrapper/test/CMakeLists.txt b/tiledb/api/c_api_support/exception_wrapper/test/CMakeLists.txt index 43841b0aa8f..f0b8845e4b0 100644 --- a/tiledb/api/c_api_support/exception_wrapper/test/CMakeLists.txt +++ b/tiledb/api/c_api_support/exception_wrapper/test/CMakeLists.txt @@ -34,3 +34,53 @@ commence(unit_test capi_exception_wrapper) # top-level dependency required for the test. capi_context_stub) conclude(unit_test) + +# +# Hook Tests +# +# These following two unit test runners are a pair, one compiled without an API +# hook compiled in, one without. They run the same test, although with different +# expectations about the results. The hook is specified in two different ways: +# * In the ordinary way, with an additional include directory +# * Specifically for testing, with a compile definition. +# The two different ways are used to verify that the hook is activated or not +# as appropriate. +# +# Changing the API hook changes the API object libraries. The API hook is +# perforce a low-level mechanism. It changes the definition of C API calls. The +# exception handler is intertwined with a few of the API classes, notably the +# handles for context and error (and config gets pulled in as a dependency). As +# a result, whereas the without-hook test runner can use an existing object +# library, the with-hook ones needs to recompile an equivalent. +# + +# Hook tests without the hook +commence(unit_test capi_ew_without_hook) + this_target_sources( + unit_capi_hook.cc + unit_capi_hook_without.cc) + this_target_object_libraries( + capi_context_stub) +conclude(unit_test) + +# Hook tests with the hook included. +commence(unit_test capi_ew_with_hook) + this_target_sources( + unit_capi_hook.cc + unit_capi_hook_with.cc + ../../../c_api/config/config_api.cc + ../../../c_api/context/context_api.cc + ../../../c_api/error/error_api.cc + ../../../../sm/storage_manager/context.cc + ../../../../sm/storage_manager/context_resources.cc + ) + this_target_link_libraries(export) + this_target_object_libraries( + exception_wrapper + storage_manager_stub + vfs + ) + target_compile_definitions(unit_capi_ew_with_hook PUBLIC WITH_HOOK) + target_include_directories( + unit_capi_ew_with_hook PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/hook) +conclude(unit_test) diff --git a/tiledb/api/c_api_support/exception_wrapper/test/hook/DIRECTORY.md b/tiledb/api/c_api_support/exception_wrapper/test/hook/DIRECTORY.md new file mode 100644 index 00000000000..b1bcef7cb37 --- /dev/null +++ b/tiledb/api/c_api_support/exception_wrapper/test/hook/DIRECTORY.md @@ -0,0 +1,5 @@ +# Directory for C API hook testing + +A user activates the C API hook by writing an override header and building the library with its directory as part of the include path. This is a compile-time mechanism, so in order to test it well, the same code should execute both with and without a test hook compiled in. + +This directory proivdes a place to put an override header that's not ordinarily included in the build. \ No newline at end of file diff --git a/tiledb/api/c_api_support/exception_wrapper/test/hook/capi_function_override.h b/tiledb/api/c_api_support/exception_wrapper/test/hook/capi_function_override.h new file mode 100644 index 00000000000..4fb9915b177 --- /dev/null +++ b/tiledb/api/c_api_support/exception_wrapper/test/hook/capi_function_override.h @@ -0,0 +1,51 @@ +/** + * @file + * tiledb/api/c_api_support/exception_wrapper/test/hook/capi_function_override.h + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2023 TileDB, Inc. + * + * 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. + * + * @section DESCRIPTION + * + * This file defines macros to define C API functions. + */ + +#ifndef TILEDB_EXCEPTION_WRAPPER_TEST_CAPI_FUNCTION_OVERRIDE_H +#define TILEDB_EXCEPTION_WRAPPER_TEST_CAPI_FUNCTION_OVERRIDE_H + +#include "../logging_aspect.h" + +#undef CAPI_PREFIX +#define CAPI_PREFIX(root) \ + template <> \ + struct CAPIFunctionMetadata { \ + static constexpr std::string_view name{"tiledb_" #root}; \ + }; + +template +struct tiledb::api::CAPIFunctionSelector { + using aspect_type = LoggingAspect; +}; + +#endif // TILEDB_EXCEPTION_WRAPPER_TEST_CAPI_FUNCTION_OVERRIDE_H diff --git a/tiledb/api/c_api_support/exception_wrapper/test/hook_common.h b/tiledb/api/c_api_support/exception_wrapper/test/hook_common.h new file mode 100644 index 00000000000..d767405f94b --- /dev/null +++ b/tiledb/api/c_api_support/exception_wrapper/test/hook_common.h @@ -0,0 +1,45 @@ +/** + * @file tiledb/api/c_api_support/exception_wrapper/test/hook_common.h + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2023 TileDB, Inc. + * + * 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. + * + * @section DESCRIPTION + * + * This file defines macros to define C API functions. + */ + +#ifndef TILEDB_EXCEPTION_WRAPPER_TEST_HOOK_COMMON_H +#define TILEDB_EXCEPTION_WRAPPER_TEST_HOOK_COMMON_H + +/* + * Convert the possible command line argument WITH_HOOK into a C++ constant. + */ +#if defined(WITH_HOOK) +constexpr bool compiled_with_hook{true}; +#else +constexpr bool compiled_with_hook{false}; +#endif + +#endif // TILEDB_EXCEPTION_WRAPPER_TEST_HOOK_COMMON_H diff --git a/tiledb/api/c_api_support/exception_wrapper/test/logging_aspect.h b/tiledb/api/c_api_support/exception_wrapper/test/logging_aspect.h new file mode 100644 index 00000000000..b8a2f78c9eb --- /dev/null +++ b/tiledb/api/c_api_support/exception_wrapper/test/logging_aspect.h @@ -0,0 +1,80 @@ +/** + * @file tiledb/api/c_api_support/exception_wrapper/test/logging_aspect.h + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2023 TileDB, Inc. + * + * 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. + * + * @section DESCRIPTION + * + * This file defines macros to define C API functions. + */ + +#ifndef TILEDB_EXCEPTION_WRAPPER_TEST_LOGGING_ASPECT_H +#define TILEDB_EXCEPTION_WRAPPER_TEST_LOGGING_ASPECT_H + +#include + +template +struct CAPIFunctionMetadata; + +template +struct CAPIFunctionMetadata; + +/** + * Base class for the logging aspect class template. + * + * This class mimics a global logger. It's rudimentary but suffices for testing. + * Instead of a growing log, it has a single "log entry" + */ +class LABase { + protected: + static std::string msg_; + static bool touched_; + + public: + static void reset() { + msg_ = ""; + touched_ = false; + } + static inline std::string message() { + return msg_; + } + static inline bool touched() { + return touched_; + } +}; + +template +class LoggingAspect; + +template +class LoggingAspect : public LABase { + public: + static void call(Args...) { + msg_ = CAPIFunctionMetadata::name; + touched_ = true; + } +}; + +#endif // TILEDB_EXCEPTION_WRAPPER_TEST_LOGGING_ASPECT_H diff --git a/tiledb/api/c_api_support/exception_wrapper/test/unit_capi_hook.cc b/tiledb/api/c_api_support/exception_wrapper/test/unit_capi_hook.cc new file mode 100644 index 00000000000..58f47540fcd --- /dev/null +++ b/tiledb/api/c_api_support/exception_wrapper/test/unit_capi_hook.cc @@ -0,0 +1,152 @@ +/** + * @file c_api_support/exception_wrapper/test/unit_capi_hook.cc + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2023 TileDB, Inc. + * + * 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. + * + * @section DESCRIPTION + */ + +#include + +#include "../../c_api_support.h" +#include "hook_common.h" +#include "logging_aspect.h" + +using namespace tiledb::api; +using namespace tiledb::api::detail; + +// from `logging_aspect.h` +std::string LABase::msg_{}; +bool LABase::touched_{false}; + +template +using selector_type = CAPIFunctionSelector; + +/** + * API function that does nothing. + */ +capi_return_t tf_null() { + return TILEDB_OK; +} +/** + * Metadata for null function. + */ +template <> +struct CAPIFunctionMetadata { + static constexpr std::string_view name{"tf_null"}; +}; + +namespace tiledb::api { +/** + * API implementation function that does nothing. + */ +capi_return_t tiledb_capi_nil(int) { + return 0; +} +} // namespace tiledb::api +/* + * API definitions with macros. + */ +CAPI_PLAIN_BEGIN(capi_nil, int x) +CAPI_PLAIN_END(capi_nil, x) + +/** + * The wrapped function with an unconditional invocation of the logging + * aspect as an explicit template argument. + */ +using null_always_wrapped_for_logging = tiledb::api:: + CAPIFunction>; +/** + * The wrapped function conditional upon an override as to whether the + * logging aspect is compiled in or not. The aspect argument is omitted, the + * default applies, and overriding is possible. + */ +using null_naybe_wrapped_for_logging = + tiledb::api::CAPIFunction; + +TEST_CASE("Compile consistency") { + /* + * In all cases verify that the default aspect is being used if and only if + * the hook is not enabled. + */ + CHECK( + compiled_with_hook != std::same_as< + selector_type::aspect_type, + CAPIFunctionNullAspect>); + if constexpr (compiled_with_hook) { + /* + * In the case "with hook", check that the aspect is what the test defines. + */ + CHECK(std::same_as< + selector_type::aspect_type, + LoggingAspect>); + } +} + +TEST_CASE("Hook unconditional") { + LABase::reset(); + CHECK(LABase::touched() == false); + CHECK(LABase::message() == ""); + tiledb::api::ExceptionAction h; + null_always_wrapped_for_logging().function(h); + CHECK(LABase::touched() == true); + CHECK(LABase::message() == "tf_null"); +} + +/* + * Test that the hook is invoked if and only if it's compiled in. This is the + * same as the previous test but for the last line. + */ +TEST_CASE("Hook conditional for touch") { + LABase::reset(); + CHECK(LABase::touched() == false); + tiledb::api::ExceptionAction h; + null_naybe_wrapped_for_logging().function(h); + CHECK(LABase::touched() == compiled_with_hook); +} + +TEST_CASE("Hook conditional with text 1") { + LABase::reset(); + CHECK(LABase::message() == ""); + tiledb::api::ExceptionAction h; + null_naybe_wrapped_for_logging().function(h); + if constexpr (compiled_with_hook) { + CHECK(LABase::message() == "tf_null"); + } else { + CHECK(LABase::message() == ""); + } +} + +TEST_CASE("Hook conditional with text 2") { + LABase::reset(); + CHECK(LABase::message() == ""); + tiledb::api::ExceptionAction h; + ::tiledb_capi_nil(0); + if constexpr (compiled_with_hook) { + CHECK(LABase::message() == "tiledb_capi_nil"); + } else { + CHECK(LABase::message() == ""); + } +} diff --git a/tiledb/api/c_api_support/exception_wrapper/test/unit_capi_hook_with.cc b/tiledb/api/c_api_support/exception_wrapper/test/unit_capi_hook_with.cc new file mode 100644 index 00000000000..31a8fc7694a --- /dev/null +++ b/tiledb/api/c_api_support/exception_wrapper/test/unit_capi_hook_with.cc @@ -0,0 +1,37 @@ +/** + * @file c_api_support/exception_wrapper/test/unit_capi_hook_with.cc + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2023 TileDB, Inc. + * + * 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. + * + * @section DESCRIPTION + */ + +#include + +#include "hook_common.h" + +TEST_CASE("Compile definition - with hook") { + CHECK(compiled_with_hook == true); +} diff --git a/tiledb/api/c_api_support/exception_wrapper/test/unit_capi_hook_without.cc b/tiledb/api/c_api_support/exception_wrapper/test/unit_capi_hook_without.cc new file mode 100644 index 00000000000..0d43505baed --- /dev/null +++ b/tiledb/api/c_api_support/exception_wrapper/test/unit_capi_hook_without.cc @@ -0,0 +1,37 @@ +/** + * @file c_api_support/exception_wrapper/test/unit_capi_hook_without.cc + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2023 TileDB, Inc. + * + * 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. + * + * @section DESCRIPTION + */ + +#include + +#include "hook_common.h" + +TEST_CASE("Compile definition - without hook") { + CHECK(compiled_with_hook == false); +}