system/serenity
system/serenity
1
0
Fork 0
mirror of https://github.com/SerenityOS/serenity synced 2024-09-30 05:05:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

AK+Everywhere: Do not implicitly copy variables in TRY macros

Browse source 
For example, consider cases where we want to propagate errors only in
specific instances:

    auto result = read_data(); // something like ErrorOr<ByteBuffer>
    if (result.is_error() && result.error().code() != EINTR)
        continue;
    auto bytes = TRY(result);

The TRY invocation will currently copy the byte buffer when the
expression (in this case, just a local variable) is stored into
_temporary_result.

This patch binds the expression to a reference to prevent such copies.
In less trival invocations (such as TRY(some_function()), this will
incur only temporary lifetime extensions, i.e. no functional change.
This commit is contained in:
Timothy Flynn 2023-02-09 13:27:43 -05:00 committed by Linus Groh
parent 4a916cd379
commit 604d5f5bca
11 changed files with 15 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
AK/Try.h
View file

@ -25,7 +25,7 @@
({ \
/* Ignore -Wshadow to allow nesting the macro. */ \
AK_IGNORE_DIAGNOSTIC("-Wshadow", \
auto _temporary_result = (expression)); \
auto&& _temporary_result = (expression)); \
static_assert(!::AK::Detail::IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
if (_temporary_result.is_error()) [[unlikely]] \
@ -37,7 +37,7 @@
({ \
/* Ignore -Wshadow to allow nesting the macro. */ \
AK_IGNORE_DIAGNOSTIC("-Wshadow", \
auto _temporary_result = (expression)); \
auto&& _temporary_result = (expression)); \
static_assert(!::AK::Detail::IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
VERIFY(!_temporary_result.is_error()); \

2
Kernel/Net/Socket.h
View file

@ -188,7 +188,7 @@ private:
// This is a special variant of TRY() that also updates the socket's SO_ERROR field on error.
#define SOCKET_TRY(expression) \
({ \
auto result = (expression); \
auto&& result = (expression); \
if (result.is_error()) \
return set_so_error(result.release_error()); \
static_assert(!::AK::Detail::IsLvalueReference<decltype(result.release_value())>, \

2
Meta/Lagom/Tools/CodeGenerators/LibLocale/GenerateLocaleData.cpp
View file

@ -233,7 +233,7 @@ struct CLDR {
// with locales such as "en-GB-oed" that are canonically invalid locale IDs.
#define TRY_OR_DISCARD(expression) \
({ \
auto _temporary_result = (expression); \
auto&& _temporary_result = (expression); \
if (_temporary_result.is_error()) \
return; \
static_assert(!::AK::Detail::IsLvalueReference<decltype(_temporary_result.release_value())>, \

2
Userland/Libraries/LibAudio/LoaderError.h
View file

@ -68,7 +68,7 @@ struct LoaderError {
// Convenience TRY-like macro to convert an Error to a LoaderError
#define LOADER_TRY(expression) \
({ \
auto _temporary_result = (expression); \
auto&& _temporary_result = (expression); \
if (_temporary_result.is_error()) \
return LoaderError(_temporary_result.release_error()); \
static_assert(!::AK::Detail::IsLvalueReference<decltype(_temporary_result.release_value())>, \

4
Userland/Libraries/LibJS/Runtime/Completion.h
View file

@ -21,7 +21,7 @@ namespace JS {
({ \
/* Ignore -Wshadow to allow nesting the macro. */ \
AK_IGNORE_DIAGNOSTIC("-Wshadow", \
auto _temporary_result = (expression)); \
auto&& _temporary_result = (expression)); \
if (_temporary_result.is_error()) { \
VERIFY(_temporary_result.error().code() == ENOMEM); \
return vm.throw_completion<JS::InternalError>(JS::ErrorType::OutOfMemory); \
@ -35,7 +35,7 @@ namespace JS {
({ \
/* Ignore -Wshadow to allow nesting the macro. */ \
AK_IGNORE_DIAGNOSTIC("-Wshadow", \
auto _temporary_result = (expression)); \
auto&& _temporary_result = (expression)); \
if (_temporary_result.is_error()) { \
auto _completion = _temporary_result.release_error(); \
\

4
Userland/Libraries/LibJS/Runtime/PromiseCapability.h
View file

@ -43,7 +43,7 @@ private:
// 27.2.1.1.1 IfAbruptRejectPromise ( value, capability ), https://tc39.es/ecma262/#sec-ifabruptrejectpromise
#define __TRY_OR_REJECT(vm, capability, expression, CALL_CHECK) \
({ \
auto _temporary_try_or_reject_result = (expression); \
auto&& _temporary_try_or_reject_result = (expression); \
/* 1. If value is an abrupt completion, then */ \
if (_temporary_try_or_reject_result.is_error()) { \
/* a. Perform ? Call(capability.[[Reject]], undefined, « value.[[Value]] »). */ \
@ -69,7 +69,7 @@ private:
// 27.2.1.1.1 IfAbruptRejectPromise ( value, capability ), https://tc39.es/ecma262/#sec-ifabruptrejectpromise
#define TRY_OR_REJECT_WITH_VALUE(vm, capability, expression) \
({ \
auto _temporary_try_or_reject_result = (expression); \
auto&& _temporary_try_or_reject_result = (expression); \
/* 1. If value is an abrupt completion, then */ \
if (_temporary_try_or_reject_result.is_error()) { \
/* a. Perform ? Call(capability.[[Reject]], undefined, « value.[[Value]] »). */ \

2
Userland/Libraries/LibVideo/DecoderError.h
View file

@ -79,7 +79,7 @@ private:
#define DECODER_TRY(category, expression) \
({ \
auto _result = ((expression)); \
auto&& _result = ((expression)); \
if (_result.is_error()) [[unlikely]] { \
auto _error_string = _result.release_error().string_literal(); \
return DecoderError::from_source_location( \

4
Userland/Libraries/LibVideo/PlaybackManager.cpp
View file

@ -15,7 +15,7 @@ namespace Video {
#define TRY_OR_FATAL_ERROR(expression) \
({ \
auto _fatal_expression = (expression); \
auto&& _fatal_expression = (expression); \
if (_fatal_expression.is_error()) { \
dispatch_fatal_error(_fatal_expression.release_error()); \
return; \
@ -156,7 +156,7 @@ bool PlaybackManager::decode_and_queue_one_sample()
#define TRY_OR_ENQUEUE_ERROR(expression) \
({ \
auto _temporary_result = ((expression)); \
auto&& _temporary_result = ((expression)); \
if (_temporary_result.is_error()) { \
dbgln_if(PLAYBACK_MANAGER_DEBUG, "Enqueued decoder error: {}", _temporary_result.error().string_literal()); \
m_frame_queue->enqueue(FrameQueueItem::error_marker(_temporary_result.release_error())); \

2
Userland/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp
View file

@ -44,7 +44,7 @@ namespace Web::Fetch::Fetching {
#define TRY_OR_IGNORE(expression) \
({ \
auto _temporary_result = (expression); \
auto&& _temporary_result = (expression); \
if (_temporary_result.is_error()) \
return; \
static_assert(!::AK::Detail::IsLvalueReference<decltype(_temporary_result.release_value())>, \

2
Userland/Libraries/LibWeb/WebDriver/ExecuteScript.cpp
View file

@ -33,7 +33,7 @@ namespace Web::WebDriver {
#define TRY_OR_JS_ERROR(expression) \
({ \
auto _temporary_result = (expression); \
auto&& _temporary_result = (expression); \
if (_temporary_result.is_error()) [[unlikely]] \
return ExecuteScriptResultType::JavaScriptError; \
static_assert(!::AK::Detail::IsLvalueReference<decltype(_temporary_result.release_value())>, \

2
Userland/Utilities/matroska.cpp
View file

@ -12,7 +12,7 @@
#define TRY_PARSE(expression) \
({ \
auto _temporary_result = ((expression)); \
auto&& _temporary_result = ((expression)); \
if (_temporary_result.is_error()) [[unlikely]] { \
outln("Encountered a parsing error: {}", _temporary_result.error().string_literal()); \
return Error::from_string_literal("Failed to parse :("); \