AK+Everywhere: Disallow returning a reference from a fallible expression

This will silently make a copy. Rather than masking this behavior, let's
explicitly disallow it.
This commit is contained in:
Timothy Flynn 2023-01-13 15:48:09 -05:00 committed by Tim Flynn
parent 3de75f6436
commit afc0e461e1
Notes: sideshowbarker 2024-07-17 01:44:58 +09:00
11 changed files with 86 additions and 54 deletions

View file

@ -15,22 +15,30 @@
// on statement expressions [1]. This is known to be implemented
// by at least clang and gcc.
// [1] https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html
//
// If the static_assert below is triggered, it means you tried to return a reference
// from a fallible expression. This will not do what you want; the statement expression
// will create a copy regardless, so it is explicitly disallowed.
#define TRY(expression) \
({ \
/* Ignore -Wshadow to allow nesting the macro. */ \
AK_IGNORE_DIAGNOSTIC("-Wshadow", \
auto _temporary_result = (expression)); \
if (_temporary_result.is_error()) [[unlikely]] \
return _temporary_result.release_error(); \
_temporary_result.release_value(); \
#define TRY(expression) \
({ \
/* Ignore -Wshadow to allow nesting the macro. */ \
AK_IGNORE_DIAGNOSTIC("-Wshadow", \
auto _temporary_result = (expression)); \
static_assert(!IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
if (_temporary_result.is_error()) [[unlikely]] \
return _temporary_result.release_error(); \
_temporary_result.release_value(); \
})
#define MUST(expression) \
({ \
/* Ignore -Wshadow to allow nesting the macro. */ \
AK_IGNORE_DIAGNOSTIC("-Wshadow", \
auto _temporary_result = (expression)); \
VERIFY(!_temporary_result.is_error()); \
_temporary_result.release_value(); \
#define MUST(expression) \
({ \
/* Ignore -Wshadow to allow nesting the macro. */ \
AK_IGNORE_DIAGNOSTIC("-Wshadow", \
auto _temporary_result = (expression)); \
static_assert(!IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
VERIFY(!_temporary_result.is_error()); \
_temporary_result.release_value(); \
})

View file

@ -184,12 +184,14 @@ 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); \
if (result.is_error()) \
return set_so_error(result.release_error()); \
result.release_value(); \
#define SOCKET_TRY(expression) \
({ \
auto result = (expression); \
if (result.is_error()) \
return set_so_error(result.release_error()); \
static_assert(!IsLvalueReference<decltype(result.release_value())>, \
"Do not return a reference from a fallible expression"); \
result.release_value(); \
})
}

View file

@ -231,12 +231,14 @@ struct CLDR {
// Some parsing is expected to fail. For example, the CLDR contains language mappings
// with locales such as "en-GB-oed" that are canonically invalid locale IDs.
#define TRY_OR_DISCARD(expression) \
({ \
auto _temporary_result = (expression); \
if (_temporary_result.is_error()) \
return; \
_temporary_result.release_value(); \
#define TRY_OR_DISCARD(expression) \
({ \
auto _temporary_result = (expression); \
if (_temporary_result.is_error()) \
return; \
static_assert(!IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
_temporary_result.release_value(); \
})
static ErrorOr<LanguageMapping> parse_language_mapping(CLDR& cldr, StringView key, StringView alias)

View file

@ -66,10 +66,12 @@ struct LoaderError {
}
// Convenience TRY-like macro to convert an Error to a LoaderError
#define LOADER_TRY(expression) \
({ \
auto _temporary_result = (expression); \
if (_temporary_result.is_error()) \
return LoaderError(_temporary_result.release_error()); \
_temporary_result.release_value(); \
#define LOADER_TRY(expression) \
({ \
auto _temporary_result = (expression); \
if (_temporary_result.is_error()) \
return LoaderError(_temporary_result.release_error()); \
static_assert(!IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
_temporary_result.release_value(); \
})

View file

@ -25,6 +25,8 @@ namespace JS {
VERIFY(_temporary_result.error().code() == ENOMEM); \
return vm.throw_completion<JS::InternalError>(JS::ErrorType::OutOfMemory); \
} \
static_assert(!IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
_temporary_result.release_value(); \
})

View file

@ -53,6 +53,9 @@ private:
return (capability)->promise(); \
} \
\
static_assert(!IsLvalueReference<decltype(_temporary_try_or_reject_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
\
/* 2. Else if value is a Completion Record, set value to value.[[Value]]. */ \
_temporary_try_or_reject_result.release_value(); \
})
@ -76,6 +79,9 @@ private:
return Value { (capability)->promise() }; \
} \
\
static_assert(!IsLvalueReference<decltype(_temporary_try_or_reject_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
\
/* 2. Else if value is a Completion Record, set value to value.[[Value]]. */ \
_temporary_try_or_reject_result.release_value(); \
})

View file

@ -77,15 +77,17 @@ private:
DeprecatedString m_description;
};
#define DECODER_TRY(category, expression) \
({ \
auto _result = ((expression)); \
if (_result.is_error()) [[unlikely]] { \
auto _error_string = _result.release_error().string_literal(); \
return DecoderError::from_source_location( \
((category)), _error_string, SourceLocation::current()); \
} \
_result.release_value(); \
#define DECODER_TRY(category, expression) \
({ \
auto _result = ((expression)); \
if (_result.is_error()) [[unlikely]] { \
auto _error_string = _result.release_error().string_literal(); \
return DecoderError::from_source_location( \
((category)), _error_string, SourceLocation::current()); \
} \
static_assert(!IsLvalueReference<decltype(_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
_result.release_value(); \
})
#define DECODER_TRY_ALLOC(expression) DECODER_TRY(DecoderErrorCategory::Memory, expression)

View file

@ -273,6 +273,8 @@ bool PlaybackManager::decode_and_queue_one_sample()
m_present_timer->start(0); \
return false; \
} \
static_assert(!IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
_temporary_result.release_value(); \
})

View file

@ -42,12 +42,14 @@
namespace Web::Fetch::Fetching {
#define TRY_OR_IGNORE(expression) \
({ \
auto _temporary_result = (expression); \
if (_temporary_result.is_error()) \
return; \
_temporary_result.release_value(); \
#define TRY_OR_IGNORE(expression) \
({ \
auto _temporary_result = (expression); \
if (_temporary_result.is_error()) \
return; \
static_assert(!IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
_temporary_result.release_value(); \
})
// https://fetch.spec.whatwg.org/#concept-fetch

View file

@ -31,12 +31,14 @@
namespace Web::WebDriver {
#define TRY_OR_JS_ERROR(expression) \
({ \
auto _temporary_result = (expression); \
if (_temporary_result.is_error()) [[unlikely]] \
return ExecuteScriptResultType::JavaScriptError; \
_temporary_result.release_value(); \
#define TRY_OR_JS_ERROR(expression) \
({ \
auto _temporary_result = (expression); \
if (_temporary_result.is_error()) [[unlikely]] \
return ExecuteScriptResultType::JavaScriptError; \
static_assert(!IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
_temporary_result.release_value(); \
})
static ErrorOr<JsonValue, ExecuteScriptResultType> internal_json_clone_algorithm(JS::Realm&, JS::Value, HashTable<JS::Object*>& seen);

View file

@ -17,6 +17,8 @@
outln("Encountered a parsing error: {}", _temporary_result.error().string_literal()); \
return Error::from_string_literal("Failed to parse :("); \
} \
static_assert(!IsLvalueReference<decltype(_temporary_result.release_value())>, \
"Do not return a reference from a fallible expression"); \
_temporary_result.release_value(); \
})