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

Annotate some functions _MSVC_CONSTEXPR #2658

Merged
merged 1 commit into from
Apr 19, 2022

Conversation

CaseyCarter
Copy link
Member

@CaseyCarter CaseyCarter commented Apr 18, 2022

Which is meaningless now, but will eventually become [[msvc::constexpr]] when VCRuntime changes (and the corresponding compiler) ship.

This mirrors internal MSVC-PR-391437.

Which is meaningless now, but will eventually become `[[msvc::constexpr]]`.

This mirrors internal MSVC-PR-391437.
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Apr 18, 2022
@CaseyCarter CaseyCarter requested a review from a team as a code owner April 18, 2022 21:06
@@ -1546,5 +1546,9 @@ compiler option, or define _ALLOW_RTCc_IN_STL to acknowledge that you have recei
#define _STL_INTERNAL_STATIC_ASSERT(...)
#endif // _ENABLE_STL_INTERNAL_CHECK

#ifndef _MSVC_CONSTEXPR // TRANSITION, VS2022v17.3p2
#define _MSVC_CONSTEXPR
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move that to the other custom attributes? Even if it has no value right now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Does it need to be now, or would a followup be ok? I plan a followup to better integrate this with #2649 (move _HAS_MSVC_ATTRIBUTE to VCRuntime so it's used consistently for all of the _MSVC_MEOW attribute macros) and I'll bring it all together then.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a hill I would die on.

It is a bit icky to add something with the goal to clean it up afterwards, but I can appreciate that it has a dependency on VCRuntime so its fine by me

Copy link
Member Author

Choose a reason for hiding this comment

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

It is a bit icky to add something with the goal to clean it up afterwards, but I can appreciate that it has a dependency on VCRuntime so its fine by me

I'm also trying to avoid repeated CI cycles as things move back-and-forth between PRs, and trying to keep the compiler Devs unblocked. This is correct but not in the final form we'd like, which I'm happy to call good enough to decouple future library work from the ongoing compiler PR.

@StephanTLavavej StephanTLavavej added the compiler Compiler work involved label Apr 18, 2022
@StephanTLavavej StephanTLavavej changed the title Annonate some functions _MSVC_CONSTEXPR Annotate some functions _MSVC_CONSTEXPR Apr 19, 2022
@StephanTLavavej StephanTLavavej merged commit 0349ce1 into microsoft:main Apr 19, 2022
@StephanTLavavej
Copy link
Member

Thanks for mirroring these compiler changes! 🪞 ⚙️ 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler Compiler work involved enhancement Something can be improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants