[Offload] Use llvm::Error throughout liboffload internals #140879
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-offload Author: Ross Brunton (RossBrunton) ChangesThis removes the Patch is 55.67 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/140879.diff 8 Files Affected:
diff --git a/offload/liboffload/include/OffloadImpl.hpp b/offload/liboffload/include/OffloadImpl.hpp
index 7d5c20bc48670..df7615a657c07 100644
--- a/offload/liboffload/include/OffloadImpl.hpp
+++ b/offload/liboffload/include/OffloadImpl.hpp
@@ -69,48 +69,31 @@ struct ErrPtrHash {
using ErrSetT = std::unordered_set<ErrPtrT, ErrPtrHash, ErrPtrEqual>;
ErrSetT &errors();
-struct ol_impl_result_t {
- ol_impl_result_t(std::nullptr_t) : Result(OL_SUCCESS) {}
- ol_impl_result_t(ol_errc_t Code) {
- if (Code == OL_ERRC_SUCCESS) {
- Result = nullptr;
- } else {
- auto Err = std::unique_ptr<ol_error_struct_t>(
- new ol_error_struct_t{Code, nullptr});
- Result = errors().emplace(std::move(Err)).first->get();
- }
- }
-
- ol_impl_result_t(ol_errc_t Code, llvm::StringRef Details) {
- assert(Code != OL_ERRC_SUCCESS);
- Result = nullptr;
- auto DetailsStr = errorStrs().insert(Details).first->getKeyData();
- auto Err = std::unique_ptr<ol_error_struct_t>(
- new ol_error_struct_t{Code, DetailsStr});
- Result = errors().emplace(std::move(Err)).first->get();
+namespace {
+ol_errc_t GetErrorCode(std::error_code Code) {
+ if (Code.category() ==
+ error::make_error_code(error::ErrorCode::SUCCESS).category()) {
+ return static_cast<ol_errc_t>(Code.value());
}
+ return OL_ERRC_UNKNOWN;
+}
+} // namespace
- static ol_impl_result_t fromError(llvm::Error &&Error) {
- ol_errc_t ErrCode;
- llvm::StringRef Details;
- llvm::handleAllErrors(std::move(Error), [&](llvm::StringError &Err) {
- ErrCode = GetErrorCode(Err.convertToErrorCode());
- Details = errorStrs().insert(Err.getMessage()).first->getKeyData();
- });
-
- return ol_impl_result_t{ErrCode, Details};
+inline ol_result_t llvmErrorToOffloadError(llvm::Error &&Err) {
+ if (!Err) {
+ // No error
+ return nullptr;
}
- operator ol_result_t() { return Result; }
+ ol_errc_t ErrCode;
+ llvm::StringRef Details;
-private:
- static ol_errc_t GetErrorCode(std::error_code Code) {
- if (Code.category() ==
- error::make_error_code(error::ErrorCode::SUCCESS).category()) {
- return static_cast<ol_errc_t>(Code.value());
- }
- return OL_ERRC_UNKNOWN;
- }
+ llvm::handleAllErrors(std::move(Err), [&](llvm::StringError &Err) {
+ ErrCode = GetErrorCode(Err.convertToErrorCode());
+ Details = errorStrs().insert(Err.getMessage()).first->getKeyData();
+ });
- ol_result_t Result;
-};
+ auto NewErr = std::unique_ptr<ol_error_struct_t>(
+ new ol_error_struct_t{ErrCode, Details.data()});
+ return errors().emplace(std::move(NewErr)).first->get();
+}
diff --git a/offload/liboffload/include/generated/OffloadEntryPoints.inc b/offload/liboffload/include/generated/OffloadEntryPoints.inc
index d70ebed934dce..9feebeea09ec3 100644
--- a/offload/liboffload/include/generated/OffloadEntryPoints.inc
+++ b/offload/liboffload/include/generated/OffloadEntryPoints.inc
@@ -7,7 +7,7 @@
//===----------------------------------------------------------------------===//
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olInit_val() {
+llvm::Error olInit_val() {
if (offloadConfig().ValidationEnabled) {
}
@@ -18,7 +18,7 @@ OL_APIEXPORT ol_result_t OL_APICALL olInit() {
llvm::errs() << "---> olInit";
}
- ol_result_t Result = olInit_val();
+ ol_result_t Result = llvmErrorToOffloadError(olInit_val());
if (offloadConfig().TracingEnabled) {
llvm::errs() << "()";
@@ -38,7 +38,7 @@ ol_result_t olInitWithCodeLoc(ol_code_location_t *CodeLocation) {
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olShutDown_val() {
+llvm::Error olShutDown_val() {
if (offloadConfig().ValidationEnabled) {
}
@@ -49,7 +49,7 @@ OL_APIEXPORT ol_result_t OL_APICALL olShutDown() {
llvm::errs() << "---> olShutDown";
}
- ol_result_t Result = olShutDown_val();
+ ol_result_t Result = llvmErrorToOffloadError(olShutDown_val());
if (offloadConfig().TracingEnabled) {
llvm::errs() << "()";
@@ -69,20 +69,23 @@ ol_result_t olShutDownWithCodeLoc(ol_code_location_t *CodeLocation) {
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olGetPlatformInfo_val(ol_platform_handle_t Platform,
- ol_platform_info_t PropName,
- size_t PropSize, void *PropValue) {
+llvm::Error olGetPlatformInfo_val(ol_platform_handle_t Platform,
+ ol_platform_info_t PropName, size_t PropSize,
+ void *PropValue) {
if (offloadConfig().ValidationEnabled) {
if (PropSize == 0) {
- return OL_ERRC_INVALID_SIZE;
+ return createOffloadError(error::ErrorCode::INVALID_SIZE,
+ "validation failure: PropSize == 0");
}
if (NULL == Platform) {
- return OL_ERRC_INVALID_NULL_HANDLE;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_HANDLE,
+ "validation failure: NULL == Platform");
}
if (NULL == PropValue) {
- return OL_ERRC_INVALID_NULL_POINTER;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_POINTER,
+ "validation failure: NULL == PropValue");
}
}
@@ -96,8 +99,8 @@ olGetPlatformInfo(ol_platform_handle_t Platform, ol_platform_info_t PropName,
llvm::errs() << "---> olGetPlatformInfo";
}
- ol_result_t Result =
- olGetPlatformInfo_val(Platform, PropName, PropSize, PropValue);
+ ol_result_t Result = llvmErrorToOffloadError(
+ olGetPlatformInfo_val(Platform, PropName, PropSize, PropValue));
if (offloadConfig().TracingEnabled) {
ol_get_platform_info_params_t Params = {&Platform, &PropName, &PropSize,
@@ -123,16 +126,18 @@ ol_result_t olGetPlatformInfoWithCodeLoc(ol_platform_handle_t Platform,
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olGetPlatformInfoSize_val(ol_platform_handle_t Platform,
- ol_platform_info_t PropName,
- size_t *PropSizeRet) {
+llvm::Error olGetPlatformInfoSize_val(ol_platform_handle_t Platform,
+ ol_platform_info_t PropName,
+ size_t *PropSizeRet) {
if (offloadConfig().ValidationEnabled) {
if (NULL == Platform) {
- return OL_ERRC_INVALID_NULL_HANDLE;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_HANDLE,
+ "validation failure: NULL == Platform");
}
if (NULL == PropSizeRet) {
- return OL_ERRC_INVALID_NULL_POINTER;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_POINTER,
+ "validation failure: NULL == PropSizeRet");
}
}
@@ -146,8 +151,8 @@ olGetPlatformInfoSize(ol_platform_handle_t Platform,
llvm::errs() << "---> olGetPlatformInfoSize";
}
- ol_result_t Result =
- olGetPlatformInfoSize_val(Platform, PropName, PropSizeRet);
+ ol_result_t Result = llvmErrorToOffloadError(
+ olGetPlatformInfoSize_val(Platform, PropName, PropSizeRet));
if (offloadConfig().TracingEnabled) {
ol_get_platform_info_size_params_t Params = {&Platform, &PropName,
@@ -172,8 +177,8 @@ ol_result_t olGetPlatformInfoSizeWithCodeLoc(ol_platform_handle_t Platform,
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olIterateDevices_val(ol_device_iterate_cb_t Callback,
- void *UserData) {
+llvm::Error olIterateDevices_val(ol_device_iterate_cb_t Callback,
+ void *UserData) {
if (offloadConfig().ValidationEnabled) {
}
@@ -185,7 +190,8 @@ olIterateDevices(ol_device_iterate_cb_t Callback, void *UserData) {
llvm::errs() << "---> olIterateDevices";
}
- ol_result_t Result = olIterateDevices_val(Callback, UserData);
+ ol_result_t Result =
+ llvmErrorToOffloadError(olIterateDevices_val(Callback, UserData));
if (offloadConfig().TracingEnabled) {
ol_iterate_devices_params_t Params = {&Callback, &UserData};
@@ -208,20 +214,23 @@ ol_result_t olIterateDevicesWithCodeLoc(ol_device_iterate_cb_t Callback,
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olGetDeviceInfo_val(ol_device_handle_t Device,
- ol_device_info_t PropName, size_t PropSize,
- void *PropValue) {
+llvm::Error olGetDeviceInfo_val(ol_device_handle_t Device,
+ ol_device_info_t PropName, size_t PropSize,
+ void *PropValue) {
if (offloadConfig().ValidationEnabled) {
if (PropSize == 0) {
- return OL_ERRC_INVALID_SIZE;
+ return createOffloadError(error::ErrorCode::INVALID_SIZE,
+ "validation failure: PropSize == 0");
}
if (NULL == Device) {
- return OL_ERRC_INVALID_NULL_HANDLE;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_HANDLE,
+ "validation failure: NULL == Device");
}
if (NULL == PropValue) {
- return OL_ERRC_INVALID_NULL_POINTER;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_POINTER,
+ "validation failure: NULL == PropValue");
}
}
@@ -236,8 +245,8 @@ OL_APIEXPORT ol_result_t OL_APICALL olGetDeviceInfo(ol_device_handle_t Device,
llvm::errs() << "---> olGetDeviceInfo";
}
- ol_result_t Result =
- olGetDeviceInfo_val(Device, PropName, PropSize, PropValue);
+ ol_result_t Result = llvmErrorToOffloadError(
+ olGetDeviceInfo_val(Device, PropName, PropSize, PropValue));
if (offloadConfig().TracingEnabled) {
ol_get_device_info_params_t Params = {&Device, &PropName, &PropSize,
@@ -262,16 +271,18 @@ ol_result_t olGetDeviceInfoWithCodeLoc(ol_device_handle_t Device,
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olGetDeviceInfoSize_val(ol_device_handle_t Device,
- ol_device_info_t PropName,
- size_t *PropSizeRet) {
+llvm::Error olGetDeviceInfoSize_val(ol_device_handle_t Device,
+ ol_device_info_t PropName,
+ size_t *PropSizeRet) {
if (offloadConfig().ValidationEnabled) {
if (NULL == Device) {
- return OL_ERRC_INVALID_NULL_HANDLE;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_HANDLE,
+ "validation failure: NULL == Device");
}
if (NULL == PropSizeRet) {
- return OL_ERRC_INVALID_NULL_POINTER;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_POINTER,
+ "validation failure: NULL == PropSizeRet");
}
}
@@ -283,7 +294,8 @@ OL_APIEXPORT ol_result_t OL_APICALL olGetDeviceInfoSize(
llvm::errs() << "---> olGetDeviceInfoSize";
}
- ol_result_t Result = olGetDeviceInfoSize_val(Device, PropName, PropSizeRet);
+ ol_result_t Result = llvmErrorToOffloadError(
+ olGetDeviceInfoSize_val(Device, PropName, PropSizeRet));
if (offloadConfig().TracingEnabled) {
ol_get_device_info_size_params_t Params = {&Device, &PropName,
@@ -308,19 +320,22 @@ ol_result_t olGetDeviceInfoSizeWithCodeLoc(ol_device_handle_t Device,
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olMemAlloc_val(ol_device_handle_t Device, ol_alloc_type_t Type,
- size_t Size, void **AllocationOut) {
+llvm::Error olMemAlloc_val(ol_device_handle_t Device, ol_alloc_type_t Type,
+ size_t Size, void **AllocationOut) {
if (offloadConfig().ValidationEnabled) {
if (Size == 0) {
- return OL_ERRC_INVALID_SIZE;
+ return createOffloadError(error::ErrorCode::INVALID_SIZE,
+ "validation failure: Size == 0");
}
if (NULL == Device) {
- return OL_ERRC_INVALID_NULL_HANDLE;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_HANDLE,
+ "validation failure: NULL == Device");
}
if (NULL == AllocationOut) {
- return OL_ERRC_INVALID_NULL_POINTER;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_POINTER,
+ "validation failure: NULL == AllocationOut");
}
}
@@ -334,7 +349,8 @@ OL_APIEXPORT ol_result_t OL_APICALL olMemAlloc(ol_device_handle_t Device,
llvm::errs() << "---> olMemAlloc";
}
- ol_result_t Result = olMemAlloc_val(Device, Type, Size, AllocationOut);
+ ol_result_t Result = llvmErrorToOffloadError(
+ olMemAlloc_val(Device, Type, Size, AllocationOut));
if (offloadConfig().TracingEnabled) {
ol_mem_alloc_params_t Params = {&Device, &Type, &Size, &AllocationOut};
@@ -358,10 +374,11 @@ ol_result_t olMemAllocWithCodeLoc(ol_device_handle_t Device,
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olMemFree_val(void *Address) {
+llvm::Error olMemFree_val(void *Address) {
if (offloadConfig().ValidationEnabled) {
if (NULL == Address) {
- return OL_ERRC_INVALID_NULL_POINTER;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_POINTER,
+ "validation failure: NULL == Address");
}
}
@@ -372,7 +389,7 @@ OL_APIEXPORT ol_result_t OL_APICALL olMemFree(void *Address) {
llvm::errs() << "---> olMemFree";
}
- ol_result_t Result = olMemFree_val(Address);
+ ol_result_t Result = llvmErrorToOffloadError(olMemFree_val(Address));
if (offloadConfig().TracingEnabled) {
ol_mem_free_params_t Params = {&Address};
@@ -394,29 +411,35 @@ ol_result_t olMemFreeWithCodeLoc(void *Address,
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olMemcpy_val(ol_queue_handle_t Queue, void *DstPtr,
- ol_device_handle_t DstDevice, void *SrcPtr,
- ol_device_handle_t SrcDevice, size_t Size,
- ol_event_handle_t *EventOut) {
+llvm::Error olMemcpy_val(ol_queue_handle_t Queue, void *DstPtr,
+ ol_device_handle_t DstDevice, void *SrcPtr,
+ ol_device_handle_t SrcDevice, size_t Size,
+ ol_event_handle_t *EventOut) {
if (offloadConfig().ValidationEnabled) {
if (Queue == NULL && EventOut != NULL) {
- return OL_ERRC_INVALID_ARGUMENT;
+ return createOffloadError(
+ error::ErrorCode::INVALID_ARGUMENT,
+ "validation failure: Queue == NULL && EventOut != NULL");
}
if (NULL == DstDevice) {
- return OL_ERRC_INVALID_NULL_HANDLE;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_HANDLE,
+ "validation failure: NULL == DstDevice");
}
if (NULL == SrcDevice) {
- return OL_ERRC_INVALID_NULL_HANDLE;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_HANDLE,
+ "validation failure: NULL == SrcDevice");
}
if (NULL == DstPtr) {
- return OL_ERRC_INVALID_NULL_POINTER;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_POINTER,
+ "validation failure: NULL == DstPtr");
}
if (NULL == SrcPtr) {
- return OL_ERRC_INVALID_NULL_POINTER;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_POINTER,
+ "validation failure: NULL == SrcPtr");
}
}
@@ -431,8 +454,8 @@ olMemcpy(ol_queue_handle_t Queue, void *DstPtr, ol_device_handle_t DstDevice,
llvm::errs() << "---> olMemcpy";
}
- ol_result_t Result =
- olMemcpy_val(Queue, DstPtr, DstDevice, SrcPtr, SrcDevice, Size, EventOut);
+ ol_result_t Result = llvmErrorToOffloadError(olMemcpy_val(
+ Queue, DstPtr, DstDevice, SrcPtr, SrcDevice, Size, EventOut));
if (offloadConfig().TracingEnabled) {
ol_memcpy_params_t Params = {&Queue, &DstPtr, &DstDevice, &SrcPtr,
@@ -459,15 +482,17 @@ ol_result_t olMemcpyWithCodeLoc(ol_queue_handle_t Queue, void *DstPtr,
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olCreateQueue_val(ol_device_handle_t Device,
- ol_queue_handle_t *Queue) {
+llvm::Error olCreateQueue_val(ol_device_handle_t Device,
+ ol_queue_handle_t *Queue) {
if (offloadConfig().ValidationEnabled) {
if (NULL == Device) {
- return OL_ERRC_INVALID_NULL_HANDLE;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_HANDLE,
+ "validation failure: NULL == Device");
}
if (NULL == Queue) {
- return OL_ERRC_INVALID_NULL_POINTER;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_POINTER,
+ "validation failure: NULL == Queue");
}
}
@@ -479,7 +504,8 @@ OL_APIEXPORT ol_result_t OL_APICALL olCreateQueue(ol_device_handle_t Device,
llvm::errs() << "---> olCreateQueue";
}
- ol_result_t Result = olCreateQueue_val(Device, Queue);
+ ol_result_t Result =
+ llvmErrorToOffloadError(olCreateQueue_val(Device, Queue));
if (offloadConfig().TracingEnabled) {
ol_create_queue_params_t Params = {&Device, &Queue};
@@ -502,10 +528,11 @@ ol_result_t olCreateQueueWithCodeLoc(ol_device_handle_t Device,
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olDestroyQueue_val(ol_queue_handle_t Queue) {
+llvm::Error olDestroyQueue_val(ol_queue_handle_t Queue) {
if (offloadConfig().ValidationEnabled) {
if (NULL == Queue) {
- return OL_ERRC_INVALID_NULL_HANDLE;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_HANDLE,
+ "validation failure: NULL == Queue");
}
}
@@ -516,7 +543,7 @@ OL_APIEXPORT ol_result_t OL_APICALL olDestroyQueue(ol_queue_handle_t Queue) {
llvm::errs() << "---> olDestroyQueue";
}
- ol_result_t Result = olDestroyQueue_val(Queue);
+ ol_result_t Result = llvmErrorToOffloadError(olDestroyQueue_val(Queue));
if (offloadConfig().TracingEnabled) {
ol_destroy_queue_params_t Params = {&Queue};
@@ -538,10 +565,11 @@ ol_result_t olDestroyQueueWithCodeLoc(ol_queue_handle_t Queue,
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olWaitQueue_val(ol_queue_handle_t Queue) {
+llvm::Error olWaitQueue_val(ol_queue_handle_t Queue) {
if (offloadConfig().ValidationEnabled) {
if (NULL == Queue) {
- return OL_ERRC_INVALID_NULL_HANDLE;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_HANDLE,
+ "validation failure: NULL == Queue");
}
}
@@ -552,7 +580,7 @@ OL_APIEXPORT ol_result_t OL_APICALL olWaitQueue(ol_queue_handle_t Queue) {
llvm::errs() << "---> olWaitQueue";
}
- ol_result_t Result = olWaitQueue_val(Queue);
+ ol_result_t Result = llvmErrorToOffloadError(olWaitQueue_val(Queue));
if (offloadConfig().TracingEnabled) {
ol_wait_queue_params_t Params = {&Queue};
@@ -574,10 +602,11 @@ ol_result_t olWaitQueueWithCodeLoc(ol_queue_handle_t Queue,
}
///////////////////////////////////////////////////////////////////////////////
-ol_impl_result_t olDestroyEvent_val(ol_event_handle_t Event) {
+llvm::Error olDestroyEvent_val(ol_event_handle_t Event) {
if (offloadConfig().ValidationEnabled) {
if (NULL == Event) {
- return OL_ERRC_INVALID_NULL_HANDLE;
+ return createOffloadError(error::ErrorCode::INVALID_NULL_HANDLE,
+ "validation failure: NULL == Event");
}
}
@@ -588,7 +617,7 @@ OL_APIEXPORT ol_result_t OL_APICALL olDestroyEvent(ol_event_handle_t Event) {
llvm::errs() << "---> olDestroyEvent";
}
- ol_result_t Result = olDestroyEvent_val(Event);
+ ol_result_t Result = l...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
callumfare
reviewed
May 21, 2025
| if (!ParamValue && !ParamValueSizeRet) { | ||
| return OL_ERRC_INVALID_NULL_POINTER; | ||
| return error::createOffloadError(error::ErrorCode::INVALID_NULL_POINTER, |
There was a problem hiding this comment.
Could we have an overload of createOffloadError that takes a ol_errc_t? That way we could use OL_ERRC_INVALID_NULL_POINTER here instead of the longer error::ErrorCode::INVALID_NULL_POINTER. The same is true of the autogenerated entry point code. It's purely subjective but that would seem tidier to me.
There was a problem hiding this comment.
There are already three overloads of createOffloadError, so this would double it to six, which feels like a lot (unless we do some fancy template shenanigans). I also think that it makes sense for C++ code to only use the C++ enums.
Of course, my views are subjective as well, and I'll add it if I'm outvoted.
There was a problem hiding this comment.
That's fair, I'm ok with leaving it then, especially if we consistently use the C++ enum
jhuber6
reviewed
May 21, 2025
Comment on lines
+83
to
86
| if (!Err) { | ||
| // No error | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
Suggested change
| if (!Err) { | |
| // No error | |
| return nullptr; | |
| } | |
| if (!Err) | |
| return nullptr; |
Shouldn't we have a success type?
There was a problem hiding this comment.
Shouldn't we have a success type?
Liboffload uses nullptr to mean success. We could change it to be a constant {OL_ERRC_SUCCESS, "success"} somewhere in memory, but I think that's out of scope for this mr and worth discussing separately.
|
@jhuber6 @callumfare Any changes you want me to make with this? |
jhuber6
reviewed
May 27, 2025
Comment on lines
+94
to
+97
| auto NewErr = std::unique_ptr<ol_error_struct_t>( | ||
| new ol_error_struct_t{ErrCode, Details.data()}); |
There was a problem hiding this comment.
Can this be make_unique intead?
There was a problem hiding this comment.
No, since ol_error_struct_t doesn't provide a constructor for this and make_unique can't accept an initialiser list (until C++20, apparently).
There was a problem hiding this comment.
Could we make a constructor for this?
There was a problem hiding this comment.
ol_error_struct_t is a C type, not a C++ type. I guess we could make a constructor and guard it with defined(__cplusplus), but that seems messy to me.
There was a problem hiding this comment.
Ah, right, forgot about the C API part. Nah, this is fine.
This removes the `ol_impl_result_t` helper class, replacing it with `llvm::Error`. In addition, some internal functions that returned `ol_errc_t` now return `llvm::Error` (with a fancy message).
jhuber6
approved these changes
May 27, 2025
|
This change seems to have introduced test failures for me: (and same in the "LTO" test variants) |
RossBrunton
added a commit
to RossBrunton/llvm-project
that referenced
this pull request
May 28, 2025
jhuber6
pushed a commit
that referenced
this pull request
May 28, 2025
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
This removes the `ol_impl_result_t` helper class, replacing it with `llvm::Error`. In addition, some internal functions that returned `ol_errc_t` now return `llvm::Error` (with a fancy message).
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the
ol_impl_result_thelper class, replacing it withllvm::Error. In addition, some internal functions that returnedol_errc_tnow returnllvm::Error(with a fancy message).