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

Return _Nontrivial_dummy_type to <optional> #2117

Merged
merged 1 commit into from
Aug 17, 2021

Conversation

CaseyCarter
Copy link
Member

We pushed this type up into <xutility> to use implementing the optional-alikes in <ranges>. After realizing that the updated C++20 constexpr rules mean we can avoid activating any union alternative in a constexpr constructor if we like, we removed the uses in <ranges>. This PR simply moves the type back to <optional> since it's no longer used elsewhere.

We pushed this type up into `<xutility>` to use implementing the `optional`-alikes in `<ranges>`. After realizing that the updated C++20 `constexpr` rules mean we can avoid activating any union alternative in a `constexpr` constructor if we like, we removed the uses in `<ranges>`. This PR simply moves the type back to `<optional>` since it's no longer used elsewhere.
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Aug 12, 2021
@CaseyCarter CaseyCarter requested a review from a team as a code owner August 12, 2021 16:59
@CaseyCarter CaseyCarter changed the title Return _Nontrivial_dummy_type to <optional> Return _Nontrivial_dummy_type to <optional> Aug 12, 2021
Copy link
Contributor

@miscco miscco left a comment

Choose a reason for hiding this comment

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

Thank for this nontrivial tidy up

Copy link
Contributor

@fsb4000 fsb4000 left a comment

Choose a reason for hiding this comment

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

Approved.

But again:

##[error]We stopped hearing from agent BUILD0003C7. Verify the agent machine is running and has a healthy network
connection. Anything that terminates an agent process, starves it for CPU, or blocks its network access can cause this error.
For more information, see: https://go.microsoft.com/fwlink/?linkid=846610

:(

@StephanTLavavej StephanTLavavej added throughput Must compile faster and removed enhancement Something can be improved labels Aug 12, 2021
@StephanTLavavej StephanTLavavej self-assigned this Aug 14, 2021
@StephanTLavavej
Copy link
Member

I'm mirroring this to an MSVC-internal PR. It's totally fine to push changes for code review feedback, but please notify me in that case.

Copy link
Member

@MahmoudGSaleh MahmoudGSaleh left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@StephanTLavavej StephanTLavavej merged commit f75c7f5 into microsoft:main Aug 17, 2021
@StephanTLavavej
Copy link
Member

Thanks for cleaning up this tiny throughput issue! 🧹 🔬 🎉

@CaseyCarter CaseyCarter deleted the dummy_type branch August 19, 2021 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
throughput Must compile faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants