LibWeb/Bindings: Support returning nullable types in dictionaries

We were previously assuming that dictionary members were always
required when being returned.

This is a bit of a weird case, because unlike _input_ dictionaries
which the spec marks as required, 'result' dictionaries do not seem to
be marked in spec IDL as required. This is still fine from the POV that
the spec is written as it states that we should only be putting the
values into the dictionary if the value exists.

We could do this through some metaprogramming constexpr type checks.
For example, if the type in our C++ representation was not an
Optional, we can skip the has_value check.

Instead of doing that, change the IDL of the result dictionaries to
annotate these members so that the IDL generator knows this
information up front. While all current cases have every single
member returned or not returned, it is conceivable that the spec
could have a situation that one member is always returned (and
should get marked as required), while the others are optionally
returned. Therefore, this new GenerateAsRequired attribute is
applied for each individual member.
This commit is contained in:
Shannon Booth 2025-01-14 00:08:27 +13:00 committed by Tim Ledbetter
parent 64a2c156bb
commit f3ec727555
Notes: github-actions[bot] 2025-02-10 17:06:24 +00:00
6 changed files with 37 additions and 30 deletions

View file

@ -30,8 +30,8 @@ enum PlaybackDirection { "normal", "reverse", "alternate", "alternate-reverse" }
// https://www.w3.org/TR/web-animations-1/#the-computedeffecttiming-dictionary
dictionary ComputedEffectTiming : EffectTiming {
unrestricted double endTime;
unrestricted double activeDuration;
[GenerateAsRequired] unrestricted double endTime;
[GenerateAsRequired] unrestricted double activeDuration;
double? localTime;
double? progress;
unrestricted double? currentIteration;

View file

@ -2,8 +2,8 @@
// https://encoding.spec.whatwg.org/#dictdef-textencoderencodeintoresult
dictionary TextEncoderEncodeIntoResult {
unsigned long long read;
unsigned long long written;
[GenerateAsRequired] unsigned long long read;
[GenerateAsRequired] unsigned long long written;
};
// https://encoding.spec.whatwg.org/#textencoder

View file

@ -45,8 +45,8 @@ dictionary NavigationReloadOptions : NavigationOptions {
};
dictionary NavigationResult {
Promise<NavigationHistoryEntry> committed;
Promise<NavigationHistoryEntry> finished;
[GenerateAsRequired] Promise<NavigationHistoryEntry> committed;
[GenerateAsRequired] Promise<NavigationHistoryEntry> finished;
};
enum NavigationHistoryBehavior {

View file

@ -42,20 +42,20 @@ dictionary URLPatternOptions {
// https://urlpattern.spec.whatwg.org/#dictdef-urlpatternresult
dictionary URLPatternResult {
sequence<URLPatternInput> inputs;
[GenerateAsRequired] sequence<URLPatternInput> inputs;
URLPatternComponentResult protocol;
URLPatternComponentResult username;
URLPatternComponentResult password;
URLPatternComponentResult hostname;
URLPatternComponentResult port;
URLPatternComponentResult pathname;
URLPatternComponentResult search;
URLPatternComponentResult hash;
[GenerateAsRequired] URLPatternComponentResult protocol;
[GenerateAsRequired] URLPatternComponentResult username;
[GenerateAsRequired] URLPatternComponentResult password;
[GenerateAsRequired] URLPatternComponentResult hostname;
[GenerateAsRequired] URLPatternComponentResult port;
[GenerateAsRequired] URLPatternComponentResult pathname;
[GenerateAsRequired] URLPatternComponentResult search;
[GenerateAsRequired] URLPatternComponentResult hash;
};
// https://urlpattern.spec.whatwg.org/#dictdef-urlpatterncomponentresult
dictionary URLPatternComponentResult {
USVString input;
record<USVString, (USVString or undefined)> groups;
[GenerateAsRequired] USVString input;
[GenerateAsRequired] record<USVString, (USVString or undefined)> groups;
};

View file

@ -25,7 +25,7 @@ dictionary AudioContextOptions {
};
dictionary AudioTimestamp {
double contextTime;
[GenerateAsRequired] double contextTime;
// FIXME: Should be DOMHighResTimeStamp, but DOMHighResTimeStamp doesn't get parsed as a double during codegen
double performanceTime;
[GenerateAsRequired] double performanceTime;
};

View file

@ -1764,7 +1764,7 @@ enum class WrappingReference {
Yes,
};
static void generate_wrap_statement(SourceGenerator& generator, ByteString const& value, IDL::Type const& type, IDL::Interface const& interface, StringView result_expression, WrappingReference wrapping_reference = WrappingReference::No, size_t recursion_depth = 0)
static void generate_wrap_statement(SourceGenerator& generator, ByteString const& value, IDL::Type const& type, IDL::Interface const& interface, StringView result_expression, WrappingReference wrapping_reference = WrappingReference::No, size_t recursion_depth = 0, bool is_optional = false)
{
auto scoped_generator = generator.fork();
scoped_generator.set("value", value);
@ -1791,7 +1791,7 @@ static void generate_wrap_statement(SourceGenerator& generator, ByteString const
return;
}
if (type.is_nullable() && !is<UnionType>(type)) {
if ((is_optional || type.is_nullable()) && !is<UnionType>(type)) {
if (type.is_string()) {
scoped_generator.append(R"~~~(
if (!@value@.has_value()) {
@ -1820,7 +1820,7 @@ static void generate_wrap_statement(SourceGenerator& generator, ByteString const
}
if (type.is_string()) {
if (type.is_nullable()) {
if (type.is_nullable() || is_optional) {
scoped_generator.append(R"~~~(
@result_expression@ JS::PrimitiveString::create(vm, @value@.release_value());
)~~~");
@ -1837,16 +1837,16 @@ static void generate_wrap_statement(SourceGenerator& generator, ByteString const
auto new_array@recursion_depth@ = MUST(JS::Array::create(realm, 0));
)~~~");
if (!type.is_nullable()) {
scoped_generator.append(R"~~~(
for (size_t i@recursion_depth@ = 0; i@recursion_depth@ < @value@.size(); ++i@recursion_depth@) {
auto& element@recursion_depth@ = @value@.at(i@recursion_depth@);
)~~~");
} else {
if (type.is_nullable() || is_optional) {
scoped_generator.append(R"~~~(
auto& @value@_non_optional = @value@.value();
for (size_t i@recursion_depth@ = 0; i@recursion_depth@ < @value@_non_optional.size(); ++i@recursion_depth@) {
auto& element@recursion_depth@ = @value@_non_optional.at(i@recursion_depth@);
)~~~");
} else {
scoped_generator.append(R"~~~(
for (size_t i@recursion_depth@ = 0; i@recursion_depth@ < @value@.size(); ++i@recursion_depth@) {
auto& element@recursion_depth@ = @value@.at(i@recursion_depth@);
)~~~");
}
@ -2024,7 +2024,14 @@ static void generate_wrap_statement(SourceGenerator& generator, ByteString const
dictionary_generator.append(R"~~~(
JS::Value @wrapped_value_name@;
)~~~");
generate_wrap_statement(dictionary_generator, ByteString::formatted("{}{}{}", value, type.is_nullable() ? "->" : ".", member.name.to_snakecase()), member.type, interface, ByteString::formatted("{} =", wrapped_value_name), WrappingReference::No, recursion_depth + 1);
// NOTE: This has similar semantics as 'required' in WebIDL. However, the spec does not put 'required' on
// _returned_ dictionary members since with the way the spec is worded it has no normative effect to
// do so. We could implement this without the 'GenerateAsRequired' extended attribute, but it would require
// the generated code to do some metaprogramming to inspect the type of the member in the C++ struct to
// determine whether the type is present or not (e.g through a has_value() on an Optional<T>, or a null
// check on a GC::Ptr<T>). So to save some complexity in the generator, give ourselves a hint of what to do.
bool is_optional = !member.extended_attributes.contains("GenerateAsRequired") && !member.default_value.has_value();
generate_wrap_statement(dictionary_generator, ByteString::formatted("{}{}{}", value, type.is_nullable() ? "->" : ".", member.name.to_snakecase()), member.type, interface, ByteString::formatted("{} =", wrapped_value_name), WrappingReference::No, recursion_depth + 1, is_optional);
dictionary_generator.append(R"~~~(
MUST(dictionary_object@recursion_depth@->create_data_property("@member_key@", @wrapped_value_name@));
@ -2057,7 +2064,7 @@ static void generate_wrap_statement(SourceGenerator& generator, ByteString const
}
}
if (type.is_nullable() && !is<UnionType>(type)) {
if ((type.is_nullable() || is_optional) && !is<UnionType>(type)) {
scoped_generator.append(R"~~~(
}
)~~~");