Skip to content

Delete noexcept on the move constructor of OrderedDict#12369

Closed
ezyang wants to merge 2 commits intopytorch:masterfrom
ezyang:pr/noexcept-better
Closed

Delete noexcept on the move constructor of OrderedDict#12369
ezyang wants to merge 2 commits intopytorch:masterfrom
ezyang:pr/noexcept-better

Conversation

@ezyang
Copy link
Contributor

@ezyang ezyang commented Oct 5, 2018

Previously we tested if default-construction was noexcept, which
doesn't really mean that the move constructor is noexcept too.

Shuts up clang-tidy.

Signed-off-by: Edward Z. Yang ezyang@fb.com

CC @goldsborough

Previously we tested if default-construction was noexcept, which
doesn't really mean that the move constructor is noexcept too.

Shuts up clang-tidy.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ezyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@ezyang
Copy link
Contributor Author

ezyang commented Oct 5, 2018

Unfortunately, this doesn't compile on Windows.

@ezyang ezyang changed the title Correctly test for noexcept-edness on the move constructor d on the move constructor Oct 5, 2018
@ezyang ezyang changed the title d on the move constructor Delete noexcept on the move constructor of OrderedDict Oct 5, 2018
@ezyang
Copy link
Contributor Author

ezyang commented Oct 5, 2018

@pytorchbot retest this please

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Sign up for free to 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.

3 participants