Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cpp1): apply memberwise semantics to base assignment #504

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

JohelEGP
Copy link
Contributor

@JohelEGP JohelEGP commented Jun 9, 2023

Resolves #402 (specifically, #402 (comment)).

Testing summary:
100% tests passed, 0 tests failed out of 684

Total Test time (real) = 113.44 sec
Acknowledgements:

@JohelEGP JohelEGP force-pushed the memberwise_base_assignment branch from f3f3339 to c950c87 Compare June 9, 2023 20:41
@JohelEGP JohelEGP force-pushed the memberwise_base_assignment branch from c950c87 to a9c7380 Compare June 20, 2023 14:23
@JohelEGP
Copy link
Contributor Author

Without this,
a base type like std::function_ref<T> can't be assigned to.
It's only copy-assignable,
and has a deleted operator=(T):

    constexpr function_ref(const function_ref&) noexcept = default;
    constexpr function_ref& operator=(const function_ref&) noexcept = default;
    template<class T> function_ref& operator=(T) = delete;

Cppfront (main branch) would end up choosing the deleted overload.
With this PR, this would work just fine.

@JohelEGP
Copy link
Contributor Author

Turns out that the deleted operator=(T) above
is constrained such that T isn't function_ref.
So you could say that
this PR is not needed,
Cppfront (main branch) is OK as is,
and the onus is on the type author to constrain their templated assignments well.

@JohelEGP
Copy link
Contributor Author

Actually, I don't think that's a valid argument against this PR.
Those templates are usually constrained such that T isn't the base type or a specialization of its template.
They are not constrained such that T isn't derived from a specialization of the base template.
So without this PR, those templates would always attempt to use the derived object.

Here I try constraining the assignment of the code in the to-be-resolved issue (and failed):
https://cpp2.godbolt.org/z/xb77dnqPf (without #486, I had to start from the generated code).
That should be the essence of what std::variant and std::optional do,
but those actually work, so I must be missing something: https://cpp2.godbolt.org/z/Ybq76YWa4.

@JohelEGP JohelEGP force-pushed the memberwise_base_assignment branch from a9c7380 to f761445 Compare July 11, 2023 01:02
@hsutter
Copy link
Owner

hsutter commented Aug 10, 2023

Looks good, thank you!

@hsutter hsutter merged commit f061ed2 into hsutter:main Aug 10, 2023
@JohelEGP JohelEGP deleted the memberwise_base_assignment branch August 10, 2023 00:40
zaucy pushed a commit to zaucy/cppfront that referenced this pull request Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants