Skip to content

Implement constexpr std::vector #1407

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

Merged
merged 53 commits into from
Feb 17, 2021
Merged

Conversation

miscco
Copy link
Contributor

@miscco miscco commented Oct 27, 2020

Fixes #45

NOTE: The first commit simply squashed the in flight PRs to simplify review

@miscco miscco requested a review from a team as a code owner October 27, 2020 21:09
@StephanTLavavej StephanTLavavej added the cxx20 C++20 feature label Oct 27, 2020
@miscco miscco force-pushed the constexpr_vector branch 3 times, most recently from bfde4ad to df5520f Compare October 28, 2020 08:47
@miscco
Copy link
Contributor Author

miscco commented Oct 28, 2020

@CaseyCarter, @StephanTLavavej I seem to recall that I found those unexplicable queue errors in one of the earlier PRs

@miscco miscco force-pushed the constexpr_vector branch 2 times, most recently from 5e8f71d to 3b81789 Compare October 28, 2020 17:46
@CaseyCarter

This comment has been minimized.

@azure-pipelines

This comment has been minimized.

@miscco miscco force-pushed the constexpr_vector branch 2 times, most recently from c38d9ab to 0905e0e Compare October 29, 2020 10:16
@miscco
Copy link
Contributor Author

miscco commented Oct 29, 2020

There were some merge conflicts with this and the previous PRs, so I went ahead and cleaned them up and dropped one.

@miscco miscco force-pushed the constexpr_vector branch 6 times, most recently from a3fd33c to 15feb28 Compare October 29, 2020 21:26
@miscco
Copy link
Contributor Author

miscco commented Nov 3, 2020

There is more of them!
https://reviews.llvm.org/D68365

@StephanTLavavej
Copy link
Member

Is this blocked on a Clang compiler bug that needs to be filed, or do you need assistance reducing the weird compiler error to a self-contained test case that can be filed?

Copy link
Contributor

@mnatsuhara mnatsuhara left a comment

Choose a reason for hiding this comment

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

🥳

@StephanTLavavej

This comment has been minimized.

@StephanTLavavej
Copy link
Member

Pushed (and mirrored) two changes:

  • yvals_core.h disables constexpr vector for Clang, but the test was defining input arrays guarded by MSVC_INTERNAL_TESTING (which is defined for all compilers internally), even though none of the following tests are active for Clang. It warns that the arrays are unused. The simplest fix is to suppress the warning, tagged with the LLVM bug number. When we can enable and test this feature for Clang, we can remove the warning suppression.
  • There were a couple occurrences of this never-defined-in-GitHub macro that were _UGLY but it is a non-ugly macro (as only test code detects it, never product code). Fixed those comments.

@StephanTLavavej

This comment has been minimized.

@azure-pipelines

This comment has been minimized.

@StephanTLavavej StephanTLavavej merged commit a31bd6e into microsoft:main Feb 17, 2021
@StephanTLavavej
Copy link
Member

Thanks for constexprizing the STL's best container, and finding tons of ways to improve compiler quality! 😹 🚀 🎉

@miscco miscco deleted the constexpr_vector branch February 17, 2021 08:00
@miscco
Copy link
Contributor Author

miscco commented Feb 17, 2021

Huge shoutout to @mnatsuhara for her incredible work on all the compiler fixes 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cxx20 C++20 feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P1004R2 constexpr std::vector
5 participants