diff --git a/AK/Optional.h b/AK/Optional.h index 4453f51d875..f62187c472c 100644 --- a/AK/Optional.h +++ b/AK/Optional.h @@ -210,7 +210,7 @@ public: } template - requires(IsConstructible && !IsSpecializationOf && !IsSpecializationOf) ALWAYS_INLINE explicit Optional(Optional const& other) + requires(IsConstructible && !IsSpecializationOf && !IsSpecializationOf && (!IsLvalueReference || IsTriviallyCopyConstructible)) ALWAYS_INLINE explicit Optional(Optional const& other) : m_has_value(other.has_value()) { if (other.has_value()) @@ -218,7 +218,7 @@ public: } template - requires(IsConstructible && !IsSpecializationOf && !IsSpecializationOf) ALWAYS_INLINE explicit Optional(Optional&& other) + requires(IsConstructible && !IsSpecializationOf && !IsSpecializationOf && (!IsLvalueReference || IsTriviallyMoveConstructible)) ALWAYS_INLINE explicit Optional(Optional&& other) : m_has_value(other.has_value()) { if (other.has_value()) @@ -503,14 +503,29 @@ public: ALWAYS_INLINE RawPtr>> operator->() const { return &value(); } ALWAYS_INLINE RawPtr> operator->() { return &value(); } - // Conversion operators from Optional -> Optional + // Conversion operators from Optional -> Optional, implicit when T is trivially copyable. ALWAYS_INLINE operator Optional>() const + requires(IsTriviallyCopyable>) { if (has_value()) return Optional>(value()); return {}; } + // Conversion operators from Optional -> Optional, explicit when T is not trivially copyable, since this is usually a mistake. + ALWAYS_INLINE explicit operator Optional>() const + requires(!IsTriviallyCopyable>) + { + if (has_value()) + return Optional>(value()); + return {}; + } + + ALWAYS_INLINE constexpr Optional> copy() const + { + return static_cast>>(*this); + } + template [[nodiscard]] ALWAYS_INLINE T value_or_lazy_evaluated(Callback callback) const { diff --git a/Libraries/LibCrypto/Certificate/Certificate.h b/Libraries/LibCrypto/Certificate/Certificate.h index b9a37884d38..e92c91806ff 100644 --- a/Libraries/LibCrypto/Certificate/Certificate.h +++ b/Libraries/LibCrypto/Certificate/Certificate.h @@ -50,17 +50,17 @@ public: return m_members.try_set(move(key), move(value)); } - Optional get(StringView key) const + Optional get(StringView key) const { return m_members.get(key); } - Optional get(ASN1::AttributeType key) const + Optional get(ASN1::AttributeType key) const { return m_members.get(enum_value(key)); } - Optional get(ASN1::ObjectClass key) const + Optional get(ASN1::ObjectClass key) const { return m_members.get(enum_value(key)); } diff --git a/Libraries/LibHTTP/HeaderMap.h b/Libraries/LibHTTP/HeaderMap.h index 5d86f0ed537..d8f97b366ce 100644 --- a/Libraries/LibHTTP/HeaderMap.h +++ b/Libraries/LibHTTP/HeaderMap.h @@ -39,7 +39,7 @@ public: return m_map.contains(name); } - [[nodiscard]] Optional get(ByteString const& name) const + [[nodiscard]] Optional get(ByteString const& name) const { return m_map.get(name); } diff --git a/Libraries/LibJS/Runtime/IndexedProperties.cpp b/Libraries/LibJS/Runtime/IndexedProperties.cpp index 2d119c66ec0..f2528a061ef 100644 --- a/Libraries/LibJS/Runtime/IndexedProperties.cpp +++ b/Libraries/LibJS/Runtime/IndexedProperties.cpp @@ -101,7 +101,7 @@ Optional GenericIndexedPropertyStorage::get(u32 index) const { if (index >= m_array_size) return {}; - return m_sparse_elements.get(index); + return m_sparse_elements.get(index).copy(); } void GenericIndexedPropertyStorage::put(u32 index, Value value, PropertyAttributes attributes) diff --git a/Libraries/LibURL/Parser.cpp b/Libraries/LibURL/Parser.cpp index f5279da2d3c..595571c2ae1 100644 --- a/Libraries/LibURL/Parser.cpp +++ b/Libraries/LibURL/Parser.cpp @@ -699,7 +699,7 @@ String Parser::percent_encode_after_encoding(TextCodec::Encoder& encoder, String } // https://url.spec.whatwg.org/#concept-basic-url-parser -URL Parser::basic_parse(StringView raw_input, Optional const& base_url, URL* url, Optional state_override, Optional encoding) +URL Parser::basic_parse(StringView raw_input, Optional base_url, URL* url, Optional state_override, Optional encoding) { dbgln_if(URL_PARSER_DEBUG, "URL::Parser::basic_parse: Parsing '{}'", raw_input); diff --git a/Libraries/LibURL/Parser.h b/Libraries/LibURL/Parser.h index c7070569e1b..6cb98bf45d7 100644 --- a/Libraries/LibURL/Parser.h +++ b/Libraries/LibURL/Parser.h @@ -58,7 +58,7 @@ public: } // https://url.spec.whatwg.org/#concept-basic-url-parser - static URL basic_parse(StringView input, Optional const& base_url = {}, URL* url = nullptr, Optional state_override = {}, Optional encoding = {}); + static URL basic_parse(StringView input, Optional base_url = {}, URL* url = nullptr, Optional state_override = {}, Optional encoding = {}); // https://url.spec.whatwg.org/#string-percent-encode-after-encoding static String percent_encode_after_encoding(TextCodec::Encoder&, StringView input, PercentEncodeSet percent_encode_set, bool space_as_plus = false); diff --git a/Libraries/LibUnicode/CurrencyCode.cpp b/Libraries/LibUnicode/CurrencyCode.cpp index 70cfbefb45f..1457f0594b8 100644 --- a/Libraries/LibUnicode/CurrencyCode.cpp +++ b/Libraries/LibUnicode/CurrencyCode.cpp @@ -197,7 +197,7 @@ static auto const& ensure_currency_codes() return currency_codes; } -Optional get_currency_code(StringView currency) +Optional get_currency_code(StringView currency) { static auto const& currency_codes = ensure_currency_codes(); return currency_codes.get(currency); diff --git a/Libraries/LibUnicode/CurrencyCode.h b/Libraries/LibUnicode/CurrencyCode.h index 8088d29b97a..11ca5f1c3d9 100644 --- a/Libraries/LibUnicode/CurrencyCode.h +++ b/Libraries/LibUnicode/CurrencyCode.h @@ -17,6 +17,6 @@ struct CurrencyCode { Optional minor_unit {}; }; -Optional get_currency_code(StringView currency); +Optional get_currency_code(StringView currency); } diff --git a/Libraries/LibWeb/CSS/CSSStyleDeclaration.h b/Libraries/LibWeb/CSS/CSSStyleDeclaration.h index 8ebc5bca0d2..be6b6765f81 100644 --- a/Libraries/LibWeb/CSS/CSSStyleDeclaration.h +++ b/Libraries/LibWeb/CSS/CSSStyleDeclaration.h @@ -29,7 +29,7 @@ public: virtual String item(size_t index) const = 0; virtual Optional property(PropertyID) const = 0; - virtual Optional custom_property(FlyString const& custom_property_name) const = 0; + virtual Optional custom_property(FlyString const& custom_property_name) const = 0; virtual WebIDL::ExceptionOr set_property(PropertyID, StringView css_text, StringView priority = ""sv); virtual WebIDL::ExceptionOr remove_property(PropertyID); @@ -76,7 +76,7 @@ public: virtual String item(size_t index) const override; virtual Optional property(PropertyID) const override; - virtual Optional custom_property(FlyString const& custom_property_name) const override { return m_custom_properties.get(custom_property_name); } + virtual Optional custom_property(FlyString const& custom_property_name) const override { return m_custom_properties.get(custom_property_name); } virtual WebIDL::ExceptionOr set_property(StringView property_name, StringView css_text, StringView priority) override; virtual WebIDL::ExceptionOr remove_property(StringView property_name) override; diff --git a/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp b/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp index 156a5f47f60..2e0dfdf94d7 100644 --- a/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp +++ b/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp @@ -601,7 +601,7 @@ Optional ResolvedCSSStyleDeclaration::property(PropertyID propert }; } -Optional ResolvedCSSStyleDeclaration::custom_property(FlyString const& name) const +Optional ResolvedCSSStyleDeclaration::custom_property(FlyString const& name) const { const_cast(m_element->document()).update_style(); diff --git a/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h b/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h index acc3c3b00bc..459965d090d 100644 --- a/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h +++ b/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h @@ -24,7 +24,7 @@ public: virtual String item(size_t index) const override; virtual Optional property(PropertyID) const override; - virtual Optional custom_property(FlyString const& custom_property_name) const override; + virtual Optional custom_property(FlyString const& custom_property_name) const override; virtual WebIDL::ExceptionOr set_property(PropertyID, StringView css_text, StringView priority) override; virtual WebIDL::ExceptionOr set_property(StringView property_name, StringView css_text, StringView priority) override; virtual WebIDL::ExceptionOr remove_property(PropertyID) override; diff --git a/Libraries/LibWeb/DOMURL/DOMURL.cpp b/Libraries/LibWeb/DOMURL/DOMURL.cpp index 0000685e72f..b5fee14210c 100644 --- a/Libraries/LibWeb/DOMURL/DOMURL.cpp +++ b/Libraries/LibWeb/DOMURL/DOMURL.cpp @@ -506,7 +506,7 @@ void strip_trailing_spaces_from_an_opaque_path(DOMURL& url) } // https://url.spec.whatwg.org/#concept-url-parser -URL::URL parse(StringView input, Optional const& base_url, Optional encoding) +URL::URL parse(StringView input, Optional base_url, Optional encoding) { // FIXME: We should probably have an extended version of URL::URL for LibWeb instead of standalone functions like this. diff --git a/Libraries/LibWeb/DOMURL/DOMURL.h b/Libraries/LibWeb/DOMURL/DOMURL.h index 7dbe174b3c6..dec456b1e91 100644 --- a/Libraries/LibWeb/DOMURL/DOMURL.h +++ b/Libraries/LibWeb/DOMURL/DOMURL.h @@ -98,6 +98,6 @@ bool host_is_domain(URL::Host const&); void strip_trailing_spaces_from_an_opaque_path(DOMURL& url); // https://url.spec.whatwg.org/#concept-url-parser -URL::URL parse(StringView input, Optional const& base_url = {}, Optional encoding = {}); +URL::URL parse(StringView input, Optional base_url = {}, Optional encoding = {}); } diff --git a/Libraries/LibWeb/Fetch/Request.cpp b/Libraries/LibWeb/Fetch/Request.cpp index dedd80d7d98..c19d8191e89 100644 --- a/Libraries/LibWeb/Fetch/Request.cpp +++ b/Libraries/LibWeb/Fetch/Request.cpp @@ -467,7 +467,7 @@ WebIDL::ExceptionOr> Request::construct_impl(JS::Realm& realm, } // 38. Let inputOrInitBody be initBody if it is non-null; otherwise inputBody. - Optional input_or_init_body = init_body + Optional input_or_init_body = init_body ? Infrastructure::Request::BodyType { *init_body } : input_body; diff --git a/Libraries/LibWeb/FileAPI/BlobURLStore.cpp b/Libraries/LibWeb/FileAPI/BlobURLStore.cpp index 48e8c3b1178..da8f50d100f 100644 --- a/Libraries/LibWeb/FileAPI/BlobURLStore.cpp +++ b/Libraries/LibWeb/FileAPI/BlobURLStore.cpp @@ -108,7 +108,7 @@ void run_unloading_cleanup_steps(GC::Ref document) } // https://w3c.github.io/FileAPI/#blob-url-resolve -Optional resolve_a_blob_url(URL::URL const& url) +Optional resolve_a_blob_url(URL::URL const& url) { // 1. Assert: url’s scheme is "blob". VERIFY(url.scheme() == "blob"sv); diff --git a/Libraries/LibWeb/FileAPI/BlobURLStore.h b/Libraries/LibWeb/FileAPI/BlobURLStore.h index 45be3b441d7..a913f05a5d5 100644 --- a/Libraries/LibWeb/FileAPI/BlobURLStore.h +++ b/Libraries/LibWeb/FileAPI/BlobURLStore.h @@ -28,7 +28,7 @@ BlobURLStore& blob_url_store(); ErrorOr generate_new_blob_url(); ErrorOr add_entry_to_blob_url_store(GC::Ref object); ErrorOr remove_entry_from_blob_url_store(StringView url); -Optional resolve_a_blob_url(URL::URL const&); +Optional resolve_a_blob_url(URL::URL const&); void run_unloading_cleanup_steps(GC::Ref); diff --git a/Libraries/LibWeb/IndexedDB/Internal/Database.cpp b/Libraries/LibWeb/IndexedDB/Internal/Database.cpp index f86d4d0c1d5..fbc4daa3f71 100644 --- a/Libraries/LibWeb/IndexedDB/Internal/Database.cpp +++ b/Libraries/LibWeb/IndexedDB/Internal/Database.cpp @@ -38,7 +38,7 @@ ConnectionQueue& ConnectionQueueHandler::for_key_and_name(StorageAPI::StorageKey }); } -Optional> Database::for_key_and_name(StorageAPI::StorageKey& key, String& name) +Optional const&> Database::for_key_and_name(StorageAPI::StorageKey& key, String& name) { return m_databases.ensure(key, [] { return HashMap>(); diff --git a/Libraries/LibWeb/IndexedDB/Internal/Database.h b/Libraries/LibWeb/IndexedDB/Internal/Database.h index d20de43d20e..4b2de0f8a32 100644 --- a/Libraries/LibWeb/IndexedDB/Internal/Database.h +++ b/Libraries/LibWeb/IndexedDB/Internal/Database.h @@ -39,7 +39,7 @@ public: return connections; } - [[nodiscard]] static Optional> for_key_and_name(StorageAPI::StorageKey&, String&); + [[nodiscard]] static Optional const&> for_key_and_name(StorageAPI::StorageKey&, String&); [[nodiscard]] static ErrorOr> create_for_key_and_name(JS::Realm&, StorageAPI::StorageKey&, String&); [[nodiscard]] static ErrorOr delete_for_key_and_name(StorageAPI::StorageKey&, String&); diff --git a/Libraries/LibWebView/CookieJar.cpp b/Libraries/LibWebView/CookieJar.cpp index cc114ebeda8..5ce475d0cbc 100644 --- a/Libraries/LibWebView/CookieJar.cpp +++ b/Libraries/LibWebView/CookieJar.cpp @@ -618,7 +618,7 @@ void CookieJar::TransientStorage::set_cookie(CookieStorageKey key, Web::Cookie:: m_dirty_cookies.set(move(key), move(cookie)); } -Optional CookieJar::TransientStorage::get_cookie(CookieStorageKey const& key) +Optional CookieJar::TransientStorage::get_cookie(CookieStorageKey const& key) { return m_cookies.get(key); } diff --git a/Libraries/LibWebView/CookieJar.h b/Libraries/LibWebView/CookieJar.h index 76494974763..b4c59a51054 100644 --- a/Libraries/LibWebView/CookieJar.h +++ b/Libraries/LibWebView/CookieJar.h @@ -43,7 +43,7 @@ class CookieJar { void set_cookies(Cookies); void set_cookie(CookieStorageKey, Web::Cookie::Cookie); - Optional get_cookie(CookieStorageKey const&); + Optional get_cookie(CookieStorageKey const&); size_t size() const { return m_cookies.size(); } diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateWindowOrWorkerInterfaces.cpp b/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateWindowOrWorkerInterfaces.cpp index 553e69880b9..96eb939cabc 100644 --- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateWindowOrWorkerInterfaces.cpp +++ b/Meta/Lagom/Tools/CodeGenerators/LibWeb/GenerateWindowOrWorkerInterfaces.cpp @@ -265,7 +265,7 @@ void add_@global_object_snake_name@_exposed_interfaces(JS::Object& global) static constexpr u8 attr = JS::Attribute::Writable | JS::Attribute::Configurable; )~~~"); - auto add_interface = [](SourceGenerator& gen, StringView name, StringView prototype_class, Optional const& legacy_constructor, Optional const& legacy_alias_name) { + auto add_interface = [](SourceGenerator& gen, StringView name, StringView prototype_class, Optional const& legacy_constructor, Optional legacy_alias_name) { gen.set("interface_name", name); gen.set("prototype_class", prototype_class); diff --git a/Tests/AK/TestOptional.cpp b/Tests/AK/TestOptional.cpp index a30e9ec3893..92a6d855740 100644 --- a/Tests/AK/TestOptional.cpp +++ b/Tests/AK/TestOptional.cpp @@ -13,6 +13,39 @@ #include #include +class NonCopyable { + AK_MAKE_NONCOPYABLE(NonCopyable); + AK_MAKE_DEFAULT_MOVABLE(NonCopyable); + +public: + NonCopyable() { } + ~NonCopyable() = default; + + int x { 13 }; +}; + +class NonTriviallyCopyable { + AK_MAKE_DEFAULT_COPYABLE(NonTriviallyCopyable); + AK_MAKE_DEFAULT_MOVABLE(NonTriviallyCopyable); + +public: + NonTriviallyCopyable() = default; + ~NonTriviallyCopyable() = default; + + ByteString x { "13" }; +}; + +class TriviallyCopyable { + AK_MAKE_DEFAULT_COPYABLE(TriviallyCopyable); + AK_MAKE_DEFAULT_MOVABLE(TriviallyCopyable); + +public: + TriviallyCopyable() = default; + ~TriviallyCopyable() = default; + + int x { 13 }; +}; + TEST_CASE(basic_optional) { Optional x; @@ -39,23 +72,12 @@ TEST_CASE(move_optional) TEST_CASE(optional_rvalue_ref_qualified_getters) { - struct DontCopyMe { - DontCopyMe() { } - ~DontCopyMe() = default; - DontCopyMe(DontCopyMe&&) = default; - DontCopyMe& operator=(DontCopyMe&&) = default; - DontCopyMe(DontCopyMe const&) = delete; - DontCopyMe& operator=(DontCopyMe const&) = delete; - - int x { 13 }; - }; - - auto make_an_optional = []() -> Optional { - return DontCopyMe {}; + auto make_an_optional = []() -> Optional { + return NonCopyable {}; }; EXPECT_EQ(make_an_optional().value().x, 13); - EXPECT_EQ(make_an_optional().value_or(DontCopyMe {}).x, 13); + EXPECT_EQ(make_an_optional().value_or(NonCopyable {}).x, 13); } TEST_CASE(optional_leak_1) @@ -299,6 +321,47 @@ static_assert(CheckAssignments, int const&>::allowed); static_assert(CheckAssignments, int&&>::allowed); // Lifetime extension static_assert(CheckAssignments, int const&&>::allowed); // Lifetime extension +static_assert(CheckAssignments, NonTriviallyCopyable&>::allowed); +static_assert(CheckAssignments, NonTriviallyCopyable const&>::allowed); +static_assert(CheckAssignments, NonTriviallyCopyable&&>::allowed); // Lifetime extension +static_assert(CheckAssignments, NonTriviallyCopyable const&&>::allowed); // Lifetime extension + +static_assert(CheckAssignments, TriviallyCopyable>::allowed); +static_assert(CheckAssignments, TriviallyCopyable const&>::allowed); +static_assert(CheckAssignments, Optional>::allowed); +static_assert(CheckAssignments, Optional>::allowed); +static_assert(CheckAssignments, Optional>::allowed); + +static_assert(CheckAssignments, NonTriviallyCopyable>::allowed); +static_assert(CheckAssignments, NonTriviallyCopyable const&>::allowed); +static_assert(CheckAssignments, Optional>::allowed); +static_assert(CheckAssignments, Optional>::allowed); +static_assert(!CheckAssignments, Optional>::allowed); + +TEST_CASE(nontrivially_copyable_assignment) +{ + { + TriviallyCopyable x {}; + Optional y = x; + Optional z = y; // Can copy implicitly + EXPECT_EQ(z->x, 13); + } + + { + NonTriviallyCopyable x {}; + Optional y = x; + Optional z = y.copy(); // Have to copy explicitly + EXPECT_EQ(z->x, "13"); + } + + { + NonTriviallyCopyable x {}; + Optional y = x; + Optional z = Optional(y); // Explicit copy constructor is still defined + EXPECT_EQ(z->x, "13"); + } +} + TEST_CASE(string_specialization) { EXPECT_EQ(sizeof(Optional), sizeof(String));