Skip to content

Commit b4416a3

Browse files
Remove class Status from exception handler. (#4393)
* Switch from inheritance to templates for the error tree visitor. Converted old visitor base class into a concept. * Adjusted C API build to take into account that the exception handler is no longer header-only. * More build adjustments * Attempt to remove a duplicate symbol linkage causing CI fail
1 parent d497fff commit b4416a3

26 files changed

+867
-257
lines changed

CMakeLists.txt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -569,15 +569,13 @@ if (TILEDB_TESTS)
569569
# C API support
570570
add_dependencies(tests unit_capi_handle unit_capi_exception_wrapper)
571571
# C API basics
572-
add_dependencies(tests unit_capi_error unit_capi_config unit_capi_context)
572+
add_dependencies(tests unit_capi_config unit_capi_context)
573573
add_dependencies(tests unit_capi_array)
574574
add_dependencies(tests unit_capi_buffer)
575575
add_dependencies(tests unit_capi_buffer_list)
576576
add_dependencies(tests unit_capi_data_order)
577577
add_dependencies(tests unit_capi_datatype)
578578
add_dependencies(tests unit_capi_filesystem)
579-
add_dependencies(tests unit_capi_object_type)
580-
add_dependencies(tests unit_capi_object_walk_order)
581579
add_dependencies(tests unit_capi_query_type)
582580
add_dependencies(tests unit_capi_vfs)
583581
# C API array schema

tiledb/api/c_api/context/context_api.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* @file tiledb/api/c_api/error/context_api.cc
2+
* @file tiledb/api/c_api/context/context_api.cc
33
*
44
* @section LICENSE
55
*

tiledb/api/c_api/context/context_api_internal.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ bool save_error(tiledb_ctx_handle_t* ctx, const tiledb::common::Status& st);
8888
* @tparam E Exception type to throw if context is not valid
8989
* @param ctx A context of unknown validity
9090
*/
91-
template <class E = CAPIStatusException>
91+
template <class E = CAPIException>
9292
inline void ensure_context_is_valid(const tiledb_ctx_handle_t* ctx) {
9393
ensure_handle_is_valid<tiledb_ctx_handle_t, E>(ctx);
9494
}

tiledb/api/c_api/data_order/CMakeLists.txt

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,17 @@
2525
#
2626

2727
include(common NO_POLICY_SCOPE)
28+
include(object_library)
2829

2930
list(APPEND SOURCES
3031
data_order_api.cc
3132
)
3233
gather_sources(${SOURCES})
3334

34-
#
35-
# Object library for other units to depend upon
36-
#
37-
add_library(capi_data_order OBJECT ${SOURCES})
38-
target_link_libraries(capi_data_order PUBLIC export)
39-
target_link_libraries(capi_data_order PUBLIC baseline $<TARGET_OBJECTS:baseline>)
40-
target_link_libraries(capi_data_order PUBLIC constants $<TARGET_OBJECTS:constants>)
41-
42-
#
43-
# Test-compile of object library ensures link-completeness
44-
#
45-
add_executable(compile_capi_data_order EXCLUDE_FROM_ALL)
46-
target_sources(compile_capi_data_order PRIVATE test/compile_capi_data_order_main.cc)
47-
target_link_libraries(compile_capi_data_order PRIVATE capi_data_order)
48-
add_dependencies(all_link_complete compile_capi_data_order)
35+
commence(object_library capi_data_order)
36+
this_target_sources(${SOURCES})
37+
this_target_link_libraries(export)
38+
this_target_object_libraries(baseline constants exception_wrapper)
39+
conclude(object_library)
4940

5041
add_test_subdirectory()

tiledb/api/c_api/datatype/CMakeLists.txt

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,28 +23,18 @@
2323
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
2424
# THE SOFTWARE.
2525
#
26-
2726
include(common NO_POLICY_SCOPE)
27+
include(object_library)
2828

2929
list(APPEND SOURCES
30-
datatype_api.cc
30+
datatype_api.cc
3131
)
3232
gather_sources(${SOURCES})
3333

34-
#
35-
# Object library for other units to depend upon
36-
#
37-
add_library(capi_datatype OBJECT ${SOURCES})
38-
target_link_libraries(capi_datatype PUBLIC export)
39-
target_link_libraries(capi_datatype PUBLIC baseline $<TARGET_OBJECTS:baseline>)
40-
target_link_libraries(capi_datatype PUBLIC constants $<TARGET_OBJECTS:constants>)
41-
42-
#
43-
# Test-compile of object library ensures link-completeness
44-
#
45-
add_executable(compile_capi_datatype EXCLUDE_FROM_ALL)
46-
target_sources(compile_capi_datatype PRIVATE test/compile_capi_datatype_main.cc)
47-
target_link_libraries(compile_capi_datatype PRIVATE capi_datatype)
48-
add_dependencies(all_link_complete compile_capi_datatype)
34+
commence(object_library capi_datatype)
35+
this_target_sources(${SOURCES})
36+
this_target_link_libraries(export)
37+
this_target_object_libraries(baseline constants capi_enumeration_stub)
38+
conclude(object_library)
4939

5040
add_test_subdirectory()

tiledb/api/c_api/error/CMakeLists.txt

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,17 @@
2525
#
2626

2727
include(common NO_POLICY_SCOPE)
28+
include(object_library)
2829

2930
list(APPEND SOURCES
3031
error_api.cc
3132
)
3233
gather_sources(${SOURCES})
3334

34-
#
35-
# Object library uses `StorageManagerStub` to support API wrappers
36-
#
37-
add_library(capi_error OBJECT ${SOURCES})
38-
target_link_libraries(capi_error PUBLIC export)
39-
target_link_libraries(capi_error PUBLIC baseline $<TARGET_OBJECTS:baseline>)
40-
41-
#
42-
# Test-compile of object library ensures link-completeness
43-
#
44-
add_executable(compile_capi_error EXCLUDE_FROM_ALL)
45-
target_link_libraries(compile_capi_error PRIVATE capi_error)
46-
target_sources(compile_capi_error PRIVATE test/compile_capi_error_main.cc)
47-
add_dependencies(all_link_complete compile_capi_error)
35+
commence(object_library capi_error)
36+
this_target_sources(${SOURCES})
37+
this_target_link_libraries(export)
38+
this_target_object_libraries(baseline exception_wrapper)
39+
conclude(object_library)
4840

4941
add_test_subdirectory()

tiledb/api/c_api/error/error_api.cc

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,6 @@
3636

3737
namespace tiledb::api {
3838

39-
bool create_error(tiledb_error_handle_t** error, const Status& st) {
40-
if (st.ok()) {
41-
return false;
42-
}
43-
*error = tiledb_error_handle_t::make_handle(st.to_string());
44-
return true;
45-
}
46-
4739
void create_error(tiledb_error_handle_t** error, const std::string& message) {
4840
*error = tiledb_error_handle_t::make_handle(message);
4941
}

tiledb/api/c_api/error/error_api_internal.h

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -72,28 +72,6 @@ struct tiledb_error_handle_t
7272
};
7373

7474
namespace tiledb::api {
75-
/**
76-
* Conditionally create a C API error object based on a `Status`.
77-
*
78-
* The error object in the C API, unlike almost every other, is created outside
79-
* a `tiledb_*_alloc` function. This function is the one that creates such
80-
* objects for other functions that can return errors.
81-
*
82-
* Note that this function can throw, since it allocates to create an error
83-
* handle. In that case the argument error is ignored and superseded by an
84-
* out-of-memory exception, which the C API wrapper will process.
85-
*
86-
* @pre `error != nullptr`. Error arguments must always be validated, because
87-
* on error they're assigned an error handle and on success they're assigned
88-
* `nullptr`.
89-
*
90-
* @param error A non-null pointer to `tiledb_error_t *` object
91-
* @param st A status that might contain an error
92-
* @return true if `st` contained an error, false if it did not
93-
*/
94-
bool create_error(
95-
tiledb_error_handle_t** error, const tiledb::common::Status& st);
96-
9775
/*
9876
* Create a C API error object with a given string.
9977
*

tiledb/api/c_api/error/test/CMakeLists.txt

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,9 @@
2323
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
2424
# THE SOFTWARE.
2525
#
26+
include(unit_test)
2627

27-
add_executable(unit_capi_error EXCLUDE_FROM_ALL)
28-
find_package(Catch_EP REQUIRED)
29-
target_link_libraries(unit_capi_error PUBLIC Catch2::Catch2WithMain)
30-
31-
# Sources for code under test
32-
target_link_libraries(unit_capi_error PUBLIC capi_error)
33-
34-
# Sources for tests
35-
target_sources(unit_capi_error PUBLIC
36-
unit_capi_error.cc
37-
)
38-
39-
add_test(
40-
NAME "unit_capi_error"
41-
COMMAND $<TARGET_FILE:unit_capi_error>
42-
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
43-
)
28+
commence(unit_test capi_error)
29+
this_target_sources(unit_capi_error.cc)
30+
this_target_object_libraries(capi_error)
31+
conclude(unit_test)

tiledb/api/c_api/filesystem/CMakeLists.txt

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,17 @@
2525
#
2626

2727
include(common NO_POLICY_SCOPE)
28+
include(object_library)
2829

2930
list(APPEND SOURCES
3031
filesystem_api.cc
3132
)
3233
gather_sources(${SOURCES})
3334

34-
#
35-
# Object library for other units to depend upon
36-
#
37-
add_library(capi_filesystem OBJECT ${SOURCES})
38-
target_link_libraries(capi_filesystem PUBLIC export)
39-
target_link_libraries(capi_filesystem PUBLIC baseline $<TARGET_OBJECTS:baseline>)
40-
target_link_libraries(capi_filesystem PUBLIC constants $<TARGET_OBJECTS:constants>)
41-
42-
#
43-
# Test-compile of object library ensures link-completeness
44-
#
45-
add_executable(compile_capi_filesystem EXCLUDE_FROM_ALL)
46-
target_sources(compile_capi_filesystem PRIVATE test/compile_capi_filesystem_main.cc)
47-
target_link_libraries(compile_capi_filesystem PRIVATE capi_filesystem)
48-
add_dependencies(all_link_complete compile_capi_filesystem)
35+
commence(object_library capi_filesystem)
36+
this_target_sources(${SOURCES})
37+
this_target_link_libraries(export)
38+
this_target_object_libraries(baseline constants exception_wrapper)
39+
conclude(object_library)
4940

5041
add_test_subdirectory()

tiledb/api/c_api/object/CMakeLists.txt

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,16 @@ gather_sources(${SOURCES})
3737
# on StorageManager that would require us to link the entire library
3838
# for this to work.
3939
#
40-
# TODO: Remove StorageManager dependency from the object APIs and
41-
# make this a real object library for unit test use.
40+
# Maturity Warning: `capi_object` can't be a full object library unit the
41+
# `StorageManager` dependency is removed. Note that the test-compile is
42+
# commented out. This is the reason that it hasn't been converted to use the
43+
# `object_library` environment for CMake.
4244
#
4345
add_library(capi_object OBJECT ${SOURCES})
4446
target_link_libraries(capi_object PUBLIC export)
4547
target_link_libraries(capi_object PUBLIC baseline $<TARGET_OBJECTS:baseline>)
4648
target_link_libraries(capi_object PUBLIC constants $<TARGET_OBJECTS:constants>)
49+
target_link_libraries(capi_object PUBLIC exception_wrapper $<TARGET_OBJECTS:exception_wrapper>)
4750

4851
#
4952
# Test-compile of object library ensures link-completeness

tiledb/api/c_api/object/test/CMakeLists.txt

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,26 +24,14 @@
2424
# THE SOFTWARE.
2525
#
2626

27-
find_package(Catch_EP REQUIRED)
27+
include(unit_test)
2828

29-
add_executable(unit_capi_object_type EXCLUDE_FROM_ALL)
30-
target_sources(unit_capi_object_type PUBLIC unit_capi_object_type.cc)
31-
target_link_libraries(unit_capi_object_type PUBLIC capi_object)
32-
target_link_libraries(unit_capi_object_type PUBLIC Catch2::Catch2WithMain)
29+
commence(unit_test capi_object_type)
30+
this_target_sources(unit_capi_object_type.cc)
31+
this_target_object_libraries(capi_object)
32+
conclude(unit_test)
3333

34-
add_test(
35-
NAME "unit_capi_object_type"
36-
COMMAND $<TARGET_FILE:unit_capi_object_type>
37-
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
38-
)
39-
40-
add_executable(unit_capi_object_walk_order EXCLUDE_FROM_ALL)
41-
target_sources(unit_capi_object_walk_order PUBLIC unit_capi_object_walk_order.cc)
42-
target_link_libraries(unit_capi_object_walk_order PUBLIC capi_object)
43-
target_link_libraries(unit_capi_object_walk_order PUBLIC Catch2::Catch2WithMain)
44-
45-
add_test(
46-
NAME "unit_capi_object_walk_order"
47-
COMMAND $<TARGET_FILE:unit_capi_object_walk_order>
48-
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
49-
)
34+
commence(unit_test capi_object_walk_order)
35+
this_target_sources(unit_capi_object_walk_order.cc)
36+
this_target_object_libraries(capi_object)
37+
conclude(unit_test)

tiledb/api/c_api/query/CMakeLists.txt

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,17 @@
2525
#
2626

2727
include(common NO_POLICY_SCOPE)
28+
include(object_library)
2829

2930
list(APPEND SOURCES
3031
query_api.cc
3132
)
3233
gather_sources(${SOURCES})
3334

34-
#
35-
# Object library for other units to depend upon
36-
#
37-
add_library(capi_query OBJECT ${SOURCES})
38-
target_link_libraries(capi_query PUBLIC export)
39-
target_link_libraries(capi_query PUBLIC baseline $<TARGET_OBJECTS:baseline>)
40-
target_link_libraries(capi_query PUBLIC constants $<TARGET_OBJECTS:constants>)
41-
42-
#
43-
# Test-compile of object library ensures link-completeness
44-
#
45-
add_executable(compile_capi_query EXCLUDE_FROM_ALL)
46-
target_sources(compile_capi_query PRIVATE test/compile_capi_query_main.cc)
47-
target_link_libraries(compile_capi_query PRIVATE capi_query)
48-
add_dependencies(all_link_complete compile_capi_query)
35+
commence(object_library capi_query)
36+
this_target_sources(${SOURCES})
37+
this_target_link_libraries(export)
38+
this_target_object_libraries(baseline constants exception_wrapper)
39+
conclude(object_library)
4940

5041
add_test_subdirectory()

tiledb/api/c_api/string/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ gather_sources(${SOURCES})
3838
commence(object_library capi_string)
3939
this_target_sources(${SOURCES})
4040
this_target_link_libraries(export)
41-
this_target_object_libraries(baseline)
41+
this_target_object_libraries(baseline capi_context_stub)
4242
conclude(object_library)
4343

4444
add_test_subdirectory()

tiledb/api/c_api_support/argument_validation.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,14 @@
3838

3939
namespace tiledb::api {
4040

41-
class CAPIStatusException : public common::StatusException {
41+
class CAPIException : public common::StatusException {
4242
public:
43-
explicit CAPIStatusException(const std::string& message)
43+
explicit CAPIException(const std::string& message)
4444
: StatusException("C API", message) {
4545
}
4646
};
47+
// Legacy alias to forestall massive sudden code change
48+
using CAPIStatusException = CAPIException;
4749

4850
/*
4951
* Validation functions
@@ -67,7 +69,7 @@ class CAPIStatusException : public common::StatusException {
6769
*/
6870
inline void ensure_output_pointer_is_valid(void* p) {
6971
if (p == nullptr) {
70-
throw CAPIStatusException("Invalid output pointer for object");
72+
throw CAPIException("Invalid output pointer for object");
7173
}
7274
}
7375

tiledb/api/c_api_support/exception_wrapper/CMakeLists.txt

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
2424
# THE SOFTWARE.
2525
#
26-
2726
include(common NO_POLICY_SCOPE)
27+
include(object_library)
2828

2929
#
3030
# `exception_wrapper` object library
@@ -34,14 +34,15 @@ include(common NO_POLICY_SCOPE)
3434
# OBJECT syntax.
3535

3636
# No actual source files at present.
37-
#list(APPEND SOURCES
38-
#)
39-
#gather_sources(${SOURCES})
37+
list(APPEND SOURCES
38+
exception_wrapper.cc
39+
)
40+
gather_sources(${SOURCES})
4041

41-
add_library(exception_wrapper OBJECT exception_wrapper.cc)
42-
target_link_libraries(exception_wrapper PUBLIC export)
43-
target_link_libraries(exception_wrapper PUBLIC baseline $<TARGET_OBJECTS:baseline>)
44-
target_link_libraries(exception_wrapper PUBLIC thread_pool $<TARGET_OBJECTS:thread_pool>)
45-
target_link_libraries(exception_wrapper PUBLIC config $<TARGET_OBJECTS:config>)
42+
commence(object_library exception_wrapper)
43+
this_target_sources(${SOURCES})
44+
this_target_link_libraries(export)
45+
this_target_object_libraries(baseline thread_pool config)
46+
conclude(object_library)
4647

4748
add_test_subdirectory()

0 commit comments

Comments
 (0)