1814683 - Support types without default constructors more consistently in IPDL

Status: RESOLVED FIXED

(Core :: IPC, enhancement)

Description

[Nika Layzell [:nika] (ni? for response)]

This builds upon the work being done in bug 1775062 to avoid the need for default constructors in all IPDL generated code, including IPDL structs, message serialization/deserialization, and async replies. Default constructors are still somewhat required by the caller for synchronous replies, as sync replies are returned by outparameter.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1814683 - Part 1: Combine parent/child fields in IPDL structs/unions, r=#ipc-reviewers!

This combines the multiple fields or variants which were previously used to
track sided types like protocol types into a single field wrapped with a
SideVariant.

This will be used in the next part to avoid the need for default constructors
for actor types allowing the proper types to be used.

Depends on D168878

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1814683 - Part 3: Use the new Maybe-based ReadParam signature throughout IPDL, r=#ipc-reviewers!

This builds on top of the work in bug 1775062, by using this new signature
everywhere, including IPDL struct members and in/outparams for IPDL messages.
This is done with relatively minimal changes by using two bindings, one of
which is a reference.

Depends on D168879

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1814683 - Part 4: Allow IPDL structs to contain non-default-constructable types, r=#ipc-reviewers!

Previously, we would always generate a default constructor for IPDL structs
which explicitly initializes every member. This would require members to have
default initializers statically. With the new approach, each member is
individually wrapped with a template parameter which will try to ensure value
initiaization, and the default constructor is implemented with = default;.

The default constructor will produce a warning on clang if it is implicitly
deleted, so that warning is also suppressed.

Depends on D168880

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1814683 - Part 5: Support serializing sequences of non-default-constructable types, r=#ipc-reviewers!

This change allows callers of ReadSequenceParam to return a
Maybe<output_iterator> rather than a T* which will be used to fill the
resulting sequence. Both STL collections and std::array have iterators like
this, which can be returned from these methods without dramatically changing
the signature to support types without default constructors.

For types which support output iterators, they will be used for any type
without a trivial default constructor to avoid unnecessary constructor calls
and initialization when deserializing.

Depends on D168881

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1814683 - Part 2: Add a constructor for IPDL structs which moves each parameter, r=#ipc-reviewers

This will be useful after part 3 where it will be used as part of implementing
Read based on Maybe for IPDL structs. Without this change, we'd need to copy to
construct an IPDL struct containing a non-default-constructable type.

Depends on D168879

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/94480b82d102
Part 1: Combine parent/child fields in IPDL structs/unions, r=ipc-reviewers,necko-reviewers,mccr8
https://hg.mozilla.org/integration/autoland/rev/b1c980c834d8
Part 2: Add a constructor for IPDL structs which moves each parameter, r=ipc-reviewers,mccr8
https://hg.mozilla.org/integration/autoland/rev/6878a1590e91
Part 3: Use the new Maybe-based ReadParam signature throughout IPDL, r=ipc-reviewers,jld
https://hg.mozilla.org/integration/autoland/rev/1706e88652d6
Part 4: Allow IPDL structs to contain non-default-constructable types, r=ipc-reviewers,jld
https://hg.mozilla.org/integration/autoland/rev/6e43042d204a
Part 5: Support serializing sequences of non-default-constructable types, r=ipc-reviewers,jld

Comment 7

[Iulian Moraru]

Backed out (bug 1607634, bug 1814683, bug 1815177, bug 1814686) for causing build bustages on MaybeStorageBase.

• Push with failures

• Failure log

• Backout link

[task 2023-03-14T22:45:19.685Z] 22:45:19     INFO -                   from Unified_cpp_dom_ipc5.cpp:2:
[task 2023-03-14T22:45:19.686Z] 22:45:19     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/MaybeStorageBase.h: In instantiation of 'constexpr mozilla::detail::MaybeStorageBase<T, true>::Union::Union(std::in_place_t, Args&& ...) [with Args = {mozilla::NotNull<mozilla::ipc::SideVariant<mozilla::dom::PBrowserParent*, mozilla::dom::PBrowserChild*> >, int}; T = mozilla::dom::PopupIPCTabContext]':
[task 2023-03-14T22:45:19.687Z] 22:45:19     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/MaybeStorageBase.h:84:61:   required from 'constexpr mozilla::detail::MaybeStorageBase<T, true>::MaybeStorageBase(std::in_place_t, Args&& ...) [with Args = {mozilla::NotNull<mozilla::ipc::SideVariant<mozilla::dom::PBrowserParent*, mozilla::dom::PBrowserChild*> >, int}; T = mozilla::dom::PopupIPCTabContext]'
[task 2023-03-14T22:45:19.688Z] 22:45:19     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Maybe.h:288:21:   required from 'constexpr mozilla::detail::MaybeStorage<T, true>::MaybeStorage(std::in_place_t, Args&& ...) [with Args = {mozilla::NotNull<mozilla::ipc::SideVariant<mozilla::dom::PBrowserParent*, mozilla::dom::PBrowserChild*> >, int}; T = mozilla::dom::PopupIPCTabContext]'
[task 2023-03-14T22:45:19.689Z] 22:45:19     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Maybe.h:378:76:   required from 'constexpr mozilla::Maybe<T>::Maybe(std::in_place_t, Args&& ...) [with Args = {mozilla::NotNull<mozilla::ipc::SideVariant<mozilla::dom::PBrowserParent*, mozilla::dom::PBrowserChild*> >, int}; T = mozilla::dom::PopupIPCTabContext]'
[task 2023-03-14T22:45:19.690Z] 22:45:19     INFO -  /builds/worker/checkouts/gecko/ipc/chromium/src/chrome/common/ipc_message_utils.h:321:58:   required from 'IPC::ReadResult<T, false>::ReadResult(std::in_place_t, Args&& ...) [with Args = {mozilla::NotNull<mozilla::ipc::SideVariant<mozilla::dom::PBrowserParent*, mozilla::dom::PBrowserChild*> >, int}; T = mozilla::dom::PopupIPCTabContext]'
[task 2023-03-14T22:45:19.690Z] 22:45:19     INFO -  /builds/worker/workspace/obj-build/ipc/ipdl/PTabContext.cpp:60:14:   required from here
[task 2023-03-14T22:45:19.691Z] 22:45:19    ERROR -  /builds/worker/workspace/obj-build/dist/include/mozilla/MaybeStorageBase.h:71:43: error: narrowing conversion of 'std::forward<int>((* & aArgs#1))' from 'int' to 'mozilla::dom::PopupIPCTabContext::uint64_t {aka long unsigned int}' inside { } [-Werror=narrowing]
[task 2023-03-14T22:45:19.691Z] 22:45:19     INFO -           : val{std::forward<Args>(aArgs)...} {}
[task 2023-03-14T22:45:19.692Z] 22:45:19     INFO -                                             ^
[task 2023-03-14T22:45:19.692Z] 22:45:19     INFO -  cc1plus: all warnings being treated as errors
[task 2023-03-14T22:45:19.693Z] 22:45:19    ERROR -  gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:670: Unified_cpp_dom_ipc5.o] Error 1
[task 2023-03-14T22:45:19.693Z] 22:45:19     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/dom/ipc'

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5709eeca48a
Part 1: Combine parent/child fields in IPDL structs/unions, r=ipc-reviewers,necko-reviewers,mccr8
https://hg.mozilla.org/integration/autoland/rev/1127139eb7a3
Part 2: Add a constructor for IPDL structs which moves each parameter, r=ipc-reviewers,mccr8
https://hg.mozilla.org/integration/autoland/rev/f892cc61d298
Part 3: Use the new Maybe-based ReadParam signature throughout IPDL, r=ipc-reviewers,jld
https://hg.mozilla.org/integration/autoland/rev/1bf4abb497b4
Part 4: Allow IPDL structs to contain non-default-constructable types, r=ipc-reviewers,jld
https://hg.mozilla.org/integration/autoland/rev/a9403d3dd996
Part 5: Support serializing sequences of non-default-constructable types, r=ipc-reviewers,jld

Comment 9

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/b5709eeca48a
https://hg.mozilla.org/mozilla-central/rev/1127139eb7a3
https://hg.mozilla.org/mozilla-central/rev/f892cc61d298
https://hg.mozilla.org/mozilla-central/rev/1bf4abb497b4
https://hg.mozilla.org/mozilla-central/rev/a9403d3dd996