[PATCH] D134475: Add C++11 attribute msvc::constexpr

Mon Mar 27 06:38:41 PDT 2023

erichkeane added a comment.

In D134475#4222805 <https://reviews.llvm.org/D134475#4222805>, @RIscRIpt wrote:

> I am sorry for protracting implementation. I am still interested to finish this (if no volunteer takes over sooner).
>
> In D134475#4070995 <https://reviews.llvm.org/D134475#4070995>, @RIscRIpt wrote:
>
>> I asked MSFT <https://developercommunity.visualstudio.com/t/msvc::constexpr-specification/10259132> to comment, let's see if they can share the spec for this attribute.
>
> They did answer! Thank you Cody! For sake of backup, I'll re-post Cody Miller's comment here
>
>> In Microsoft Developer Community / msvc::constexpr-specification <https://developercommunity.visualstudio.com/t/msvc::constexpr-specification/10259132>, **Cody Miller** wrote:
>> Hello!
>>
>> We added `[[msvc::constexpr]]` to support `std::construct_at` and `std::ranges::construct_at`. These functions are used in lieu of placement new in constexpr contexts to support the forms of dynamic memory allocation that C++20 supports.
>>
>> We originally implemented this by intercepting the names of the invoked functions and implemented them within the compiler. We (the compiler team) ultimately weren’t satisfied with this approach because it added complexity to the frontend and led to some surprising behavior (such as users implementing their own `std::construct_at` (technically invoking undefined-behavior, I think) and seeing behavior that may have differed from their implementation).
>>
>> We added the attribute to allow limited contexts to invoke placement new during constexpr evaluation, which allowed the compiler to handle the standard library’s implementation of the two special functions mentioned above.
>>
>> The semantics are (from memory on a Sunday evening, apologies for unclear or forgotten explanations):
>>
>> - A function may be either `[[msvc::constexpr]]`, `constexpr`, or neither, but no combination of these is allowed.
>> - A function annotated with `[[msvc::constexpr]]` is unofficially called an “extended constexpr” function and may call other `constexpr` or extended constexpr functions.
>> - An extended constexpr function has the same restrictions as a constexpr function, except when otherwise noted here.
>> - A return statement may be annotated with `[[msvc::constexpr]]` to allow its containing constexpr function to call an extended constexpr function.
>> - An extended constexpr function can call other extended constexpr functions without a statement-level annotation.
>> - A constexpr function may use placement new within a return statement annotated with `[[msvc::constexpr]]`.
>>
>> We aimed to add a general mechanism to support this feature, but we only are officially supporting it for use in `std::construct_at` and `std::ranges::construct_at` at the moment – feel free to file bugs if you find issues through experimentation, though!
>>
>> The restriction to the `return` statement is reflective of us wanting to keep the scope of the attribute small.
>>
>> Here’s a godbolt link that hopefully clearly captures the above.
>>
>> We use the annotation (behind the `_MSVC_CONSTEXPR` macro) in our STL implementation in two <https://github.com/microsoft/STL/blob/10722961aba982f0a551c0a81de55a1cb832ba01/stl/inc/xutility#L252> places <https://github.com/microsoft/STL/blob/c42483d733f588571119ff46f9816a15506cd9fb/stl/inc/memory#L540>.
>>
>> Hope this explanation helps! Let me know if anything is unclear.
>
> Currently I am lingered to decide how to implement checks for constant evaluation in clang/lib/AST/ExprConstant.cpp <https://github.com/llvm/llvm-project/blob/08d094a0e457360ad8b94b017d2dc277e697ca76/clang/lib/AST/ExprConstant.cpp>.
> At the moment I have two ideas:
> Idea 1: Using helper RAII classes like `BlockScopeRAII` keep track of `AttributedStmt` and `ReturnStmt` in `EvalInfo` and/or `CallStackFrame`; then in CheckConstexprFunction <https://github.com/llvm/llvm-project/blob/08d094a0e457360ad8b94b017d2dc277e697ca76/clang/lib/AST/ExprConstant.cpp#L5530> check whether above rules are followed. However I don't see that `EvalInfo` or `CallStackFrame` allow storing references to arbitrary statements, and extending these classes only to support `[[msvc::constexpr]]` does not sound like a reasonable idea.
> Idea 2: Pass `Expr* CallerExpr` to CheckConstexprFunction <https://github.com/llvm/llvm-project/blob/08d094a0e457360ad8b94b017d2dc277e697ca76/clang/lib/AST/ExprConstant.cpp#L5530>, and then traverse parents of `CallerExpr` to check that above rules are followed. This would require implementing a simple `ParentVisitor` class which would use `ParentMapContext`. I'm not completely sure about this idea, because I see that `ParentMapContext` is used seldom and its docs mention about "opportunities for optimization" (sounds like performance problems).
>
> By the way, it feels weird that we have to implement complex checks (for attributed return statement) only to limit the possibilities of constant evaluation; on the other hand without these checks clang won't match MSVC implementation.
>
> P.S. Based on Cody's comment and experiments in Compiler Explorer, I made a summary with three rules; however for some reason, in the evening, I cannot replicate them anymore; once I have time and figure it out, I'll post an update. Note: STL has `[[msvc::constexpr]] void* operator new(size_t, void*)`.

I don't have a good idea, I don't think any sort of RAII object is the right way (since it will be wrong on child calls/etc), and the ParentMap is definitely not the right way.  Perhaps just a flag for CheckConstexprFunction?  I've not spent too much time in this section of the compiler, so hopefully someone else can come along and help. I suspect it is a property of `CallStackFrame`?  But that already contains a link to the FucntionDecl, right?  As far as the 'on return statement', I think the bit on CallStackFrame/EvalInfo is probably what is necessary.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134475/new/

https://reviews.llvm.org/D134475