Skip to content
This repository was archived by the owner on Aug 8, 2023. It is now read-only.

Commit 0c90386

Browse files
kkaeferkkaefer
committed
WIP: use expected<T, E> for passing on errors
1 parent 03790e7 commit 0c90386

18 files changed

+148
-168
lines changed

‎CMakeLists.txt

+5-3
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,10 @@ if(WITH_NODEJS AND COMMAND mbgl_platform_node)
176176
endif()
177177

178178
if(CMAKE_GENERATOR STREQUAL "Xcode")
179-
write_xcconfig_target_properties(
180-
mbgl-core
181-
mbgl-filesource
179+
set_xcconfig_target_properties(mbgl-core)
180+
set_xcconfig_target_properties(mbgl-filesource)
181+
file(GENERATE
182+
OUTPUT "${CMAKE_BINARY_DIR}/config.xcconfig"
183+
INPUT "${CMAKE_SOURCE_DIR}/scripts/config.xcconfig.in"
182184
)
183185
endif()

‎bin/offline.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,9 @@ int main(int argc, char *argv[]) {
136136

137137
std::signal(SIGINT, [] (int) { stop(); });
138138

139-
fileSource.createOfflineRegion(definition, metadata, [&] (std::exception_ptr error, optional<OfflineRegion> region_) {
140-
if (error) {
141-
std::cerr << "Error creating region: " << util::toString(error) << std::endl;
139+
fileSource.createOfflineRegion(definition, metadata, [&] (mbgl::expected<OfflineRegion, std::exception_ptr> region_) {
140+
if (!region_) {
141+
std::cerr << "Error creating region: " << util::toString(region_.error()) << std::endl;
142142
loop.stop();
143143
exit(1);
144144
} else {

‎cmake/core-files.cmake

+1
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,7 @@ set(MBGL_CORE_FILES
664664
include/mbgl/util/enum.hpp
665665
include/mbgl/util/event.hpp
666666
include/mbgl/util/exception.hpp
667+
include/mbgl/util/expected.hpp
667668
include/mbgl/util/feature.hpp
668669
include/mbgl/util/font_stack.hpp
669670
include/mbgl/util/geo.hpp

‎cmake/filesource.cmake

+3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
add_vendor_target(expected INTERFACE)
2+
13
add_library(mbgl-filesource STATIC
24
# File source
35
include/mbgl/storage/default_file_source.hpp
@@ -39,6 +41,7 @@ target_include_directories(mbgl-filesource
3941

4042
target_link_libraries(mbgl-filesource
4143
PUBLIC mbgl-core
44+
PUBLIC expected
4245
)
4346

4447
mbgl_filesource()

‎cmake/mbgl.cmake

+17-38
Original file line numberDiff line numberDiff line change
@@ -115,13 +115,15 @@ endfunction()
115115

116116
# Creates a library target for a vendored dependency
117117
function(add_vendor_target NAME TYPE)
118-
add_library(${NAME} ${TYPE} "${CMAKE_CURRENT_SOURCE_DIR}/cmake/empty.cpp")
119118
set(INCLUDE_TYPE "INTERFACE")
120119
set(SOURCE_TYPE "INTERFACE")
121120
if (TYPE STREQUAL "STATIC" OR TYPE STREQUAL "SHARED")
121+
add_library(${NAME} ${TYPE} "${CMAKE_CURRENT_SOURCE_DIR}/cmake/empty.cpp")
122122
set(INCLUDE_TYPE "PUBLIC")
123123
set(SOURCE_TYPE "PRIVATE")
124124
set_target_properties(${NAME} PROPERTIES SOURCES "")
125+
else()
126+
add_library(${NAME} ${TYPE})
125127
endif()
126128
set_target_properties(${NAME} PROPERTIES INTERFACE_SOURCES "")
127129
file(STRINGS "${CMAKE_CURRENT_SOURCE_DIR}/vendor/${NAME}/files.txt" FILES)
@@ -137,48 +139,25 @@ macro(set_xcode_property TARGET XCODE_PROPERTY XCODE_VALUE)
137139
set_property(TARGET ${TARGET} PROPERTY XCODE_ATTRIBUTE_${XCODE_PROPERTY} ${XCODE_VALUE})
138140
endmacro (set_xcode_property)
139141

140-
function(_get_xcconfig_property target var)
141-
get_property(result TARGET ${target} PROPERTY INTERFACE_${var} SET)
142-
if (result)
143-
get_property(result TARGET ${target} PROPERTY INTERFACE_${var})
144-
if (var STREQUAL "LINK_LIBRARIES")
145-
# Remove target names from the list of linker flags, since Xcode can't deal with them.
146-
set(link_flags)
147-
foreach(item IN LISTS result)
148-
if (NOT TARGET ${item})
149-
list(APPEND link_flags ${item})
150-
endif()
151-
endforeach()
152-
set(result "${link_flags}")
142+
function(set_xcconfig_target_properties target)
143+
# Create a list of linked libraries for use in the xcconfig generation script.
144+
get_property(result TARGET ${target} PROPERTY INTERFACE_LINK_LIBRARIES)
145+
string(GENEX_STRIP "${result}" result)
146+
# Remove target names from the list of linker flags, since Xcode can't deal with them.
147+
set(link_flags)
148+
foreach(item IN LISTS result)
149+
if (NOT TARGET ${item})
150+
list(APPEND link_flags ${item})
153151
endif()
154-
string(REPLACE ";-framework " ";-framework;" result "${result}")
155-
string(REPLACE ";" "\" \"" result "${result}")
156-
string(REPLACE "-" "_" target "${target}")
157-
set(${target}_${var} "${result}" PARENT_SCOPE)
158-
endif()
159-
endfunction()
160-
161-
if(MBGL_PLATFORM STREQUAL "ios")
162-
execute_process(
163-
COMMAND git submodule update --init platform/ios/vendor/mapbox-events-ios
164-
WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}")
165-
endif()
166-
167-
function(write_xcconfig_target_properties)
168-
foreach(target ${ARGN})
169-
_get_xcconfig_property(${target} INCLUDE_DIRECTORIES)
170-
_get_xcconfig_property(${target} LINK_LIBRARIES)
171152
endforeach()
172-
configure_file(
173-
"${CMAKE_SOURCE_DIR}/scripts/config.xcconfig.in"
174-
"${CMAKE_BINARY_DIR}/config.xcconfig"
175-
@ONLY
176-
)
153+
string(REPLACE ";-framework " ";-framework;" link_flags "${link_flags}")
154+
string(REPLACE ";" "\" \"" link_flags "${link_flags}")
155+
set_xcode_property(${target} XCCONFIG_LINK_LIBRARIES "${link_flags}")
177156
endfunction()
178157

179158
# Set Xcode project build settings to be consistent with the CXX flags we're
180159
# using. (Otherwise, Xcode's defaults may override some of these.)
181-
macro(initialize_xcode_cxx_build_settings target)
160+
function(initialize_xcode_cxx_build_settings target)
182161
# -Wall
183162
set_xcode_property(${target} GCC_WARN_SIGN_COMPARE YES)
184163
set_xcode_property(${target} GCC_WARN_UNINITIALIZED_AUTOS YES)
@@ -206,7 +185,7 @@ macro(initialize_xcode_cxx_build_settings target)
206185

207186
# -flto
208187
set_xcode_property(${target} LLVM_LTO $<$<OR:$<CONFIG:Release>,$<CONFIG:RelWithDebugInfo>>:YES>)
209-
endmacro(initialize_xcode_cxx_build_settings)
188+
endfunction()
210189

211190
# CMake 3.1 does not have this yet.
212191
set(CMAKE_CXX14_STANDARD_COMPILE_OPTION "-std=c++14")

‎include/mbgl/storage/default_file_source.hpp

+7-8
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <mbgl/storage/offline.hpp>
66
#include <mbgl/util/constants.hpp>
77
#include <mbgl/util/optional.hpp>
8+
#include <mbgl/util/expected.hpp>
89

910
#include <vector>
1011
#include <mutex>
@@ -55,8 +56,7 @@ class DefaultFileSource : public FileSource {
5556
* callback, which will be executed on the database thread; it is the responsibility
5657
* of the SDK bindings to re-execute a user-provided callback on the main thread.
5758
*/
58-
void listOfflineRegions(std::function<void (std::exception_ptr,
59-
optional<std::vector<OfflineRegion>>)>);
59+
void listOfflineRegions(std::function<void (expected<OfflineRegions, std::exception_ptr>)>);
6060

6161
/*
6262
* Create an offline region in the database.
@@ -71,16 +71,14 @@ class DefaultFileSource : public FileSource {
7171
*/
7272
void createOfflineRegion(const OfflineRegionDefinition& definition,
7373
const OfflineRegionMetadata& metadata,
74-
std::function<void (std::exception_ptr,
75-
optional<OfflineRegion>)>);
74+
std::function<void (expected<OfflineRegion, std::exception_ptr>)>);
7675

7776
/*
7877
* Update an offline region metadata in the database.
7978
*/
8079
void updateOfflineMetadata(const int64_t regionID,
8180
const OfflineRegionMetadata& metadata,
82-
std::function<void (std::exception_ptr,
83-
optional<OfflineRegionMetadata>)>);
81+
std::function<void (expected<OfflineRegionMetadata, std::exception_ptr>)>);
8482
/*
8583
* Register an observer to be notified when the state of the region changes.
8684
*/
@@ -97,8 +95,9 @@ class DefaultFileSource : public FileSource {
9795
* executed on the database thread; it is the responsibility of the SDK bindings
9896
* to re-execute a user-provided callback on the main thread.
9997
*/
100-
void getOfflineRegionStatus(OfflineRegion&, std::function<void (std::exception_ptr,
101-
optional<OfflineRegionStatus>)>) const;
98+
void getOfflineRegionStatus(
99+
OfflineRegion&,
100+
std::function<void (expected<OfflineRegionStatus, std::exception_ptr>)>) const;
102101

103102
/*
104103
* Remove an offline region from the database and perform any resources evictions

‎include/mbgl/storage/offline.hpp

+2
Original file line numberDiff line numberDiff line change
@@ -210,4 +210,6 @@ class OfflineRegion {
210210
const OfflineRegionMetadata metadata;
211211
};
212212

213+
using OfflineRegions = std::vector<OfflineRegion>;
214+
213215
} // namespace mbgl

‎platform/android/config.cmake

+1
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ macro(mbgl_platform_core)
9797

9898
target_link_libraries(mbgl-core
9999
PRIVATE nunicode
100+
PUBLIC expected
100101
PUBLIC -llog
101102
PUBLIC -landroid
102103
PUBLIC -ljnigraphics

‎platform/android/src/offline/offline_manager.cpp

+13-10
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,18 @@ void OfflineManager::listOfflineRegions(jni::JNIEnv& env_, jni::Object<FileSourc
2626
//Keep a shared ptr to a global reference of the callback and file source so they are not GC'd in the meanwhile
2727
callback = std::shared_ptr<jni::jobject>(callback_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()),
2828
jFileSource = std::shared_ptr<jni::jobject>(jFileSource_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter())
29-
](std::exception_ptr error, mbgl::optional<std::vector<mbgl::OfflineRegion>> regions) mutable {
29+
](mbgl::expected<mbgl::OfflineRegions, std::exception_ptr> regions) mutable {
3030

3131
// Reattach, the callback comes from a different thread
3232
android::UniqueEnv env = android::AttachEnv();
3333

34-
if (error) {
35-
OfflineManager::ListOfflineRegionsCallback::onError(*env, jni::Object<ListOfflineRegionsCallback>(*callback), error);
36-
} else if (regions) {
37-
OfflineManager::ListOfflineRegionsCallback::onList(*env, jni::Object<FileSource>(*jFileSource), jni::Object<ListOfflineRegionsCallback>(*callback), std::move(regions));
34+
if (regions) {
35+
OfflineManager::ListOfflineRegionsCallback::onList(
36+
*env, jni::Object<FileSource>(*jFileSource),
37+
jni::Object<ListOfflineRegionsCallback>(*callback), std::move(*regions));
38+
} else {
39+
OfflineManager::ListOfflineRegionsCallback::onError(
40+
*env, jni::Object<ListOfflineRegionsCallback>(*callback), regions.error());
3841
}
3942
});
4043
}
@@ -59,19 +62,19 @@ void OfflineManager::createOfflineRegion(jni::JNIEnv& env_,
5962
//Keep a shared ptr to a global reference of the callback and file source so they are not GC'd in the meanwhile
6063
callback = std::shared_ptr<jni::jobject>(callback_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter()),
6164
jFileSource = std::shared_ptr<jni::jobject>(jFileSource_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter())
62-
](std::exception_ptr error, mbgl::optional<mbgl::OfflineRegion> region) mutable {
65+
](mbgl::expected<mbgl::OfflineRegion, std::exception_ptr> region) mutable {
6366

6467
// Reattach, the callback comes from a different thread
6568
android::UniqueEnv env = android::AttachEnv();
6669

67-
if (error) {
68-
OfflineManager::CreateOfflineRegionCallback::onError(*env, jni::Object<CreateOfflineRegionCallback>(*callback), error);
69-
} else if (region) {
70+
if (region) {
7071
OfflineManager::CreateOfflineRegionCallback::onCreate(
7172
*env,
7273
jni::Object<FileSource>(*jFileSource),
73-
jni::Object<CreateOfflineRegionCallback>(*callback), std::move(region)
74+
jni::Object<CreateOfflineRegionCallback>(*callback), std::move(*region)
7475
);
76+
} else {
77+
OfflineManager::CreateOfflineRegionCallback::onError(*env, jni::Object<CreateOfflineRegionCallback>(*callback), region.error());
7578
}
7679
});
7780
}

‎platform/android/src/offline/offline_region.cpp

+10-10
Original file line numberDiff line numberDiff line change
@@ -107,14 +107,14 @@ void OfflineRegion::getOfflineRegionStatus(jni::JNIEnv& env_, jni::Object<Offlin
107107
fileSource.getOfflineRegionStatus(*region, [
108108
//Ensure the object is not gc'd in the meanwhile
109109
callback = std::shared_ptr<jni::jobject>(callback_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter())
110-
](std::exception_ptr error, mbgl::optional<mbgl::OfflineRegionStatus> status) mutable {
110+
](mbgl::expected<mbgl::OfflineRegionStatus, std::exception_ptr> status) mutable {
111111
// Reattach, the callback comes from a different thread
112112
android::UniqueEnv env = android::AttachEnv();
113113

114-
if (error) {
115-
OfflineRegionStatusCallback::onError(*env, jni::Object<OfflineRegionStatusCallback>(*callback), error);
116-
} else if (status) {
117-
OfflineRegionStatusCallback::onStatus(*env, jni::Object<OfflineRegionStatusCallback>(*callback), std::move(status));
114+
if (status) {
115+
OfflineRegionStatusCallback::onStatus(*env, jni::Object<OfflineRegionStatusCallback>(*callback), std::move(*status));
116+
} else {
117+
OfflineRegionStatusCallback::onError(*env, jni::Object<OfflineRegionStatusCallback>(*callback), status.error());
118118
}
119119
});
120120
}
@@ -144,14 +144,14 @@ void OfflineRegion::updateOfflineRegionMetadata(jni::JNIEnv& env_, jni::Array<jn
144144
fileSource.updateOfflineMetadata(region->getID(), metadata, [
145145
//Ensure the object is not gc'd in the meanwhile
146146
callback = std::shared_ptr<jni::jobject>(callback_.NewGlobalRef(env_).release()->Get(), GenericGlobalRefDeleter())
147-
](std::exception_ptr error, mbgl::optional<mbgl::OfflineRegionMetadata> data) mutable {
147+
](mbgl::expected<mbgl::OfflineRegionMetadata, std::exception_ptr> data) mutable {
148148
// Reattach, the callback comes from a different thread
149149
android::UniqueEnv env = android::AttachEnv();
150150

151-
if (error) {
152-
OfflineRegionUpdateMetadataCallback::onError(*env, jni::Object<OfflineRegionUpdateMetadataCallback>(*callback), error);
153-
} else if (data) {
154-
OfflineRegionUpdateMetadataCallback::onUpdate(*env, jni::Object<OfflineRegionUpdateMetadataCallback>(*callback), std::move(data));
151+
if (data) {
152+
OfflineRegionUpdateMetadataCallback::onUpdate(*env, jni::Object<OfflineRegionUpdateMetadataCallback>(*callback), std::move(*data));
153+
} else {
154+
OfflineRegionUpdateMetadataCallback::onError(*env, jni::Object<OfflineRegionUpdateMetadataCallback>(*callback), data.error());
155155
}
156156
});
157157
}

‎platform/darwin/src/MGLOfflinePack.mm

+1-1
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ - (void)requestProgress {
141141
mbgl::DefaultFileSource *mbglFileSource = [[MGLOfflineStorage sharedOfflineStorage] mbglFileSource];
142142

143143
__weak MGLOfflinePack *weakSelf = self;
144-
mbglFileSource->getOfflineRegionStatus(*_mbglOfflineRegion, [&, weakSelf](__unused std::exception_ptr exception, mbgl::optional<mbgl::OfflineRegionStatus> status) {
144+
mbglFileSource->getOfflineRegionStatus(*_mbglOfflineRegion, [&, weakSelf](mbgl::expected<mbgl::OfflineRegionStatus, std::exception_ptr> status) {
145145
if (status) {
146146
mbgl::OfflineRegionStatus checkedStatus = *status;
147147
dispatch_async(dispatch_get_main_queue(), ^{

‎platform/darwin/src/MGLOfflineStorage.mm

+12-12
Original file line numberDiff line numberDiff line change
@@ -288,16 +288,16 @@ - (void)_addPackForRegion:(id <MGLOfflineRegion>)region withContext:(NSData *)co
288288
const mbgl::OfflineTilePyramidRegionDefinition regionDefinition = [(id <MGLOfflineRegion_Private>)region offlineRegionDefinition];
289289
mbgl::OfflineRegionMetadata metadata(context.length);
290290
[context getBytes:&metadata[0] length:metadata.size()];
291-
self.mbglFileSource->createOfflineRegion(regionDefinition, metadata, [&, completion](std::exception_ptr exception, mbgl::optional<mbgl::OfflineRegion> mbglOfflineRegion) {
291+
self.mbglFileSource->createOfflineRegion(regionDefinition, metadata, [&, completion](mbgl::expected<mbgl::OfflineRegion, std::exception_ptr> mbglOfflineRegion) {
292292
NSError *error;
293-
if (exception) {
294-
NSString *errorDescription = @(mbgl::util::toString(exception).c_str());
293+
if (!mbglOfflineRegion) {
294+
NSString *errorDescription = @(mbgl::util::toString(mbglOfflineRegion.error()).c_str());
295295
error = [NSError errorWithDomain:MGLErrorDomain code:-1 userInfo:errorDescription ? @{
296296
NSLocalizedDescriptionKey: errorDescription,
297297
} : nil];
298298
}
299299
if (completion) {
300-
MGLOfflinePack *pack = mbglOfflineRegion ? [[MGLOfflinePack alloc] initWithMBGLRegion:new mbgl::OfflineRegion(std::move(*mbglOfflineRegion))] : nil;
300+
MGLOfflinePack *pack = mbglOfflineRegion ? [[MGLOfflinePack alloc] initWithMBGLRegion:new mbgl::OfflineRegion(std::move(mbglOfflineRegion.value()))] : nil;
301301
dispatch_async(dispatch_get_main_queue(), [&, completion, error, pack](void) {
302302
completion(pack, error);
303303
});
@@ -347,17 +347,17 @@ - (void)reloadPacks {
347347
}
348348

349349
- (void)getPacksWithCompletionHandler:(void (^)(NSArray<MGLOfflinePack *> *packs, NSError * _Nullable error))completion {
350-
self.mbglFileSource->listOfflineRegions([&, completion](std::exception_ptr exception, mbgl::optional<std::vector<mbgl::OfflineRegion>> regions) {
350+
self.mbglFileSource->listOfflineRegions([&, completion](mbgl::expected<mbgl::OfflineRegions, std::exception_ptr> result) {
351351
NSError *error;
352-
if (exception) {
352+
NSMutableArray *packs;
353+
if (!result) {
353354
error = [NSError errorWithDomain:MGLErrorDomain code:-1 userInfo:@{
354-
NSLocalizedDescriptionKey: @(mbgl::util::toString(exception).c_str()),
355+
NSLocalizedDescriptionKey: @(mbgl::util::toString(result.error()).c_str()),
355356
}];
356-
}
357-
NSMutableArray *packs;
358-
if (regions) {
359-
packs = [NSMutableArray arrayWithCapacity:regions->size()];
360-
for (mbgl::OfflineRegion &region : *regions) {
357+
} else {
358+
auto& regions = result.value();
359+
packs = [NSMutableArray arrayWithCapacity:regions.size()];
360+
for (auto &region : regions) {
361361
MGLOfflinePack *pack = [[MGLOfflinePack alloc] initWithMBGLRegion:new mbgl::OfflineRegion(std::move(region))];
362362
[packs addObject:pack];
363363
}

0 commit comments

Comments
 (0)