Skip to content

Avoid explicitly resetting a std::[experimental::]optional - #874

Merged
dean0x7d merged 1 commit into
pybind:masterfrom
bmerry:optional-noreset
May 27, 2017
Merged

Avoid explicitly resetting a std::[experimental::]optional#874
dean0x7d merged 1 commit into
pybind:masterfrom
bmerry:optional-noreset

Conversation

@bmerry

Copy link
Copy Markdown
Contributor

Now that #851 has removed all multiple uses of a caster, it can just use
the default-constructed value with needing a reset. This fixes two
issues:

  1. With std::experimental::optional (at least under GCC 5.4), the = {}
    would construct an instance of the optional type and then move-assign
    it, which fails if the value type isn't move-assignable.

  2. With older versions of Boost, the = {} could fail because it is
    ambiguous, allowing construction of either boost::none or the value
    type.

Now that pybind#851 has removed all multiple uses of a caster, it can just use
the default-constructed value with needing a reset. This fixes two
issues:
1. With std::experimental::optional (at least under GCC 5.4), the `= {}`
would construct an instance of the optional type and then move-assign
it, which fails if the value type isn't move-assignable.
2. With older versions of Boost, the `= {}` could fail because it is
ambiguous, allowing construction of either `boost::none` or the value
type.
@bmerry

Copy link
Copy Markdown
ContributorAuthor

This is intended as an alternative to #850.

@dean0x7d

Copy link
Copy Markdown
Member

Thanks for simplifying this! Looks good together with the updated casters. The single-use rule is manually enforced right now, but it should become a compiler-enforced feature with #864.

@dean0x7d
dean0x7d merged commit 46dbee7 into pybind:masterMay 27, 2017
@bmerry
bmerry deleted the optional-noreset branch May 28, 2017 10:52
@dean0x7ddean0x7d modified the milestone: v2.2Aug 13, 2017
@rwgkrwgk mentioned this pull request Feb 9, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@bmerry@dean0x7d