Bug 236246 - Non-default constructible, non-copy-assignable types cannot be decoded with IPC::Decoder
Summary: Non-default constructible, non-copy-assignable types cannot be decoded with I...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-07 10:03 PST by Kimmo Kinnunen
Modified: 2022-02-10 01:29 PST (History)
6 users (show)

See Also:


Attachments
Patch (7.35 KB, patch)
2022-02-07 11:21 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kimmo Kinnunen 2022-02-07 10:03:43 PST
Non-default constructible types cannot be decoded with IPC::Decoder
Comment 1 Kimmo Kinnunen 2022-02-07 11:21:09 PST
Created attachment 451127 [details]
Patch
Comment 2 Sam Weinig 2022-02-07 16:41:34 PST
I am curious, what is the error you get when using a non-default constructible type?
Comment 3 Kimmo Kinnunen 2022-02-07 22:55:07 PST
It's same as this

https://godbolt.org/z/87e49njsK

#include <optional>

struct WriteReference {
    const int version;
    const int reads;
    WriteReference(int v, int r) : version(v), reads(r) { }
};
int main(int, char**) {
    std::optional<WriteReference> o { std::in_place, 1, 2 };
    std::optional<WriteReference> r { WriteReference { 1, 2 }};
    std::optional<WriteReference> r2;
    r2 = o;

    return 0;
}


ource>:12:8: error: object of type 'std::optional<WriteReference>' cannot be assigned because its copy assignment operator is implicitly deleted
    r2 = o;
       ^
/opt/compiler-explorer/clang-11.0.1/bin/../include/c++/v1/optional:767:41: note: explicitly defaulted function was implicitly deleted here
    _LIBCPP_INLINE_VISIBILITY optional& operator=(const optional&) = default;
                                        ^
/opt/compiler-explorer/clang-11.0.1/bin/../include/c++/v1/optional:587:7: note: copy assignment operator of 'optional<WriteReference>' is implicitly deleted because base class '__optional_sfinae_assign_base_t<WriteReference>' (aka '__sfinae_assign_base<(is_copy_constructible<WriteReference>::value && is_copy_assignable<WriteReference>::value), (is_move_constructible<WriteReference>::value && is_move_assignable<WriteReference>::value)>') has a deleted copy assignment operator
    , private __optional_sfinae_assign_base_t<_Tp>
      ^
/opt/compiler-explorer/clang-11.0.1/bin/../include/c++/v1/__tuple:528:25: note: 'operator=' has been explicitly marked deleted here
  __sfinae_assign_base& operator=(__sfinae_assign_base const&) = delete;
                
    ^
Comment 4 Kimmo Kinnunen 2022-02-07 22:59:06 PST
And to just avoid the discussion, I'll rename the bug to non-default constructible and non-copy-assignable. However, naturally even though the type would be copy-assignable it cannot be decoded if it is non-default constructible. This is because in order to copy over via assignment, std::optional first default-constructs something to copy over.
Comment 5 Sam Weinig 2022-02-08 08:16:10 PST
(In reply to Kimmo Kinnunen from comment #4)
> And to just avoid the discussion, I'll rename the bug to non-default
> constructible and non-copy-assignable. However, naturally even though the
> type would be copy-assignable it cannot be decoded if it is non-default
> constructible. This is because in order to copy over via assignment,
> std::optional first default-constructs something to copy over.

This all seems fine, I am more curious than anything. Would using .emplace() also resolve the issue?
Comment 6 Sam Weinig 2022-02-08 08:16:50 PST
(In reply to Sam Weinig from comment #5)
> (In reply to Kimmo Kinnunen from comment #4)
> > And to just avoid the discussion, I'll rename the bug to non-default
> > constructible and non-copy-assignable. However, naturally even though the
> > type would be copy-assignable it cannot be decoded if it is non-default
> > constructible. This is because in order to copy over via assignment,
> > std::optional first default-constructs something to copy over.
> 
> This all seems fine, I am more curious than anything. Would using .emplace()
> also resolve the issue?

And just to be clear, I am not arguing it would be better, just thinking out loud through questions.
Comment 7 Kimmo Kinnunen 2022-02-08 08:28:31 PST
(In reply to Sam Weinig from comment #5)
> This all seems fine, I am more curious than anything. Would using .emplace()
> also resolve the issue?

r2.emplace(std::move(*o)) when T is move-constructible.
Comment 8 EWS 2022-02-10 01:28:20 PST
Committed r289524 (247055@main): <https://commits.webkit.org/247055@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 451127 [details].
Comment 9 Radar WebKit Bug Importer 2022-02-10 01:29:16 PST
<rdar://problem/88742816>