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

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt updated this revision to Diff 515740.
RIscRIpt added a comment.

A proper implementation

In D134475#4224082 , @erichkeane 
wrote:

> 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.



> But that already contains a link to the FucntionDecl, right?

Yes, but we need to know current context (whether we're in `[[msvc::constexpr]] 
return` statement).

> As far as the 'on return statement', I think the bit on 
> CallStackFrame/EvalInfo is probably what is necessary.

That's what I ended-up doing. This was the most reasonable approach I could 
come-up with.

I added a property to `CallStackFrame`, because it's a property of current 
function: whether we are in valid context which permits evaluation of 
`[[msvc::constexpr]]`. Additionally I added `MSConstexprContextRAII` which is 
used in `case Stmt::AttributedStmtClass:` to alter the property.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/msvc-attrs-invalid.cpp
  clang/test/AST/msvc-attrs.cpp
  clang/test/AST/msvc-constexpr-new.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -83,6 +83,7 @@
 // CHECK-NEXT: LoaderUninitialized (SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: Lockable (SubjectMatchRule_record)
 // CHECK-NEXT: MIGServerRoutine (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_block)
+// CHECK-NEXT: MSConstexpr (SubjectMatchRule_function)
 // CHECK-NEXT: MSStruct (SubjectMatchRule_record)
 // CHECK-NEXT: MaybeUndef (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: MicroMips (SubjectMatchRule_function)
Index: clang/test/AST/msvc-constexpr-new.cpp
===
--- /dev/null
+++ clang/test/AST/msvc-constexpr-new.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fms-extensions -std=c++20 -ast-dump %s | FileCheck %s
+
+// CHECK: used operator new
+// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} 
+[[nodiscard]] [[msvc::constexpr]] inline void* __cdecl operator new(decltype(sizeof(void*)), void* p) noexcept { return p; }
+
+// CHECK: used constexpr construct_at
+// CHECK: AttributedStmt 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} 
+constexpr int* construct_at(int* p, int v) { [[msvc::constexpr]] return ::new (p) int(v); }
+
+constexpr bool check_construct_at() { int x; return *construct_at(&x, 42) == 42; }
+
+static_assert(check_construct_at());
Index: clang/test/AST/msvc-attrs.cpp
===
--- /dev/null
+++ clang/test/AST/msvc-attrs.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -fms-extensions -std=c++20 -ast-dump %s | FileCheck %s
+
+// [[msvc::constexpr]] tests
+
+// MSConstexprDocs (1)
+// CHECK: used f1 'bool ()'
+// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} 
+[[msvc::constexpr]] bool f1() { return true; }
+
+// MSConstexprDocs (2)
+// CHECK: used constexpr f2 'bool ()'
+// CHECK: AttributedStmt 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} 
+constexpr bool f2() { [[msvc::constexpr]] return f1(); }
+
+static_assert(f2());
+
+constexpr bool f3() { [[msvc::constexpr]] return f1() || f1() && f1(); }
+static_assert(f3());
+
+[[msvc::constexpr]] int f4(int x) { [[msvc::constexpr]] return x > 1 ? 1 + f4(x / 2) : 0; }
+constexpr bool f5() { [[msvc::constexpr]] return f4(32) == 5; }
+static_assert(f5());
+
+[[msvc::constexpr]] int f6(int x)
+{
+switch (x)
+{
+case 42: return 1337;
+default:
+ if (x < 0) [[msvc::constexpr]] return f4(-x);
+ else return x;
+}
+}
+
+constexpr bool f7() { [[msvc::constexpr]] return f6(f6(42) - 133

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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In 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 , @RIscRIpt 
> wrote:
>
>> I asked MSFT 
>> 
>>  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 
>> ,
>>  **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 
>> 
>>  places 
>> .
>>
>> 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 
> .
> 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 
> 
>  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 
> ,
>  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)

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

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt added a comment.

I am sorry for protracting implementation. I am still interested to finish this 
(if no volunteer takes over sooner).

In D134475#4070995 , @RIscRIpt wrote:

> I asked MSFT 
> 
>  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 
> ,
>  **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 
> 
>  places 
> .
>
> 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 
.
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 

 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 
,
 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 chec

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

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt added a comment.

I asked MSFT 

 to comment, let's see if they can share the spec for this attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134475#3829583 , @RIscRIpt wrote:

>> TODO: I think, I'll need to read more about constexpr for functions in 
>> standard (and LLVM code), to add relevant restrictions here.
>
> In the standard I was able to find the following paragraphs about `constexpr`:
>
> 1. Neither constructor nor destructor can be constexpr if the class has some 
> virtual base class; this holds for `[[msvc::constexpr]]` in MSVC 14.33
> 2. `constexpr` is not a part of function signature, so same non-constexpr 
> function cannot be defined. The same holds for C++11 attributes, and 
> `[[msvc::constexpr]]` in MSVC 14.33
>
> I will add two relevant tests in `msvc-attrs-invalid.cpp`

Good, thank you!

> Regarding LLVM code, I made the following observations:
>
> - `FunctionDecl::isConstexprSpecified()` is mostly used for printing code, in 
> other words, to check that the function is `constexpr` (and not `consteval`).

Sort of. A templated function marked with `constexpr` remains a constexpr 
function even if it can't be executed at compile time, so 
`isConstexprSpecified()` is really telling you "did the user write `constexpr` 
on this declaration?" instead of "is this function really constexpr?" 
(http://eel.is/c++draft/dcl.constexpr#4)

> - `FunctionDecl::getConstexprKind()` is used by:
> - `ASTImporter.cpp` - seems that, it should take care of attributes 
> independently from the `getConstexprKind()`
> - `ASTWriterDecl.cpp` - seems that, it should take care of attributes 
> independently from the `getConstexprKind()`
> - `SemaDecl.cpp` / `SemaDeclCXX.cpp` - checks that re-declaration was not 
> performed with different constexpr specifier. MSVC and clang with current 
> implementation allows re-declaration with different attributes (there are 
> tests with `f2-f5` in `msvc-attrs.cpp`)
> - `SemaDeclCXX` - enables C++ constexpr constructor inheritance - problem - 
> my current implementation supports it, whereas MSVC doesn't:
>
>   // Check '[[msvc::constexpr]]' is not taken into account during constructor 
> inheritance
>   struct s2 {
>   [[msvc::constexpr]] s2() {}
>   constexpr operator bool() { return false; };
>   };
>   
>   struct s3 : s2 {
>   constexpr operator bool() { return true; };
>   };
>   static_assert(s3()); // MSVC: C2131: expression did not evaluate to a 
> constant
>   static_assert(s3()); // clang with my patch: OK

Hmmm, that's interesting! I wonder if that's intentional behavior for MSVC or a 
bug. I'm guessing there are no open issues on Microsoft's bug tracker because 
this isn't a documented feature? Might be worth filing one to see what they 
think though. (Might not be worth it either, see below.)

> - `SemaTemplate.cpp` - enables C++ template re-specialization with different 
> constexpr specifier, not a problem, because we are allowed to do the same 
> with attributes (don't we?)

Shake a magic 8-ball, unfortunately. There's a core issue open currently about 
template specializations and attributes: http://wg21.link/cwg2604

> - `SemaTemplateInstantiateDecl.cpp` (`TemplateDeclInstantiator`) - enables 
> C++ template instantiation with same constexpr specifier
>
>   template
>   struct s1 {
>   [[msvc::constexpr]] s1() {}
>   constexpr operator bool() { return true; };
>   };
>   
>   static_assert(s1()); // MSVC: C2131: expression did not evaluate to a 
> constant
>   static_assert(s1()); // clang with my patch: OK

Can you do: `constexpr s1 s;` in MSVC or does that also fail? (e.g., is 
the issue with the constructor or with the conversion operator?)

> - `TreeTransform.h` - code is related to lambdas - out-of-scope of the 
> current patch.
>
> Regarding `MSVC:C2131` I found one more problem:
>
>   [[msvc::constexpr]] int C() { return 0; }
>   constexpr int V = C(); // MSVC: C2131: expression did not evaluate to a 
> constant
>   constexpr int W = C(); // clang with my patch: OK

What the heck, that's a surprise! I would have expected that to work the same 
in Clang and MSVC (accepted by both).

> I am starting to question my patch - does LLVM really need it, if
>
> 1. There's no documentation for `[[msvc::constexpr]]`
> 2. The trivial implementation makes it an alias, but the semantic behavior is 
> different - we could fix that, but there are lots of things to check
>
> @aaron.ballman WDYT?
>
> P.S. I'd rather abandon it, than trying to match behavior of MSVC.

If you don't want to work on the feature, that's totally fine (you're under no 
obligation to finish this if it turns out to be more than you wanted to take 
on)! I think we're going to need to support this attribute at some point 
because `` has `#define _MSVC_CONSTEXPR [[msvc::constexpr]]` and 
that macro is used by the STL headers. But the usage I am seeing there is even 
more interesting than what we've discovered so far. From ``:

  constexpr _Ty* operator()(_Ty* _Location, _Types&&... _Args) const
  noexcept(noexcept(::new (const_cast(st

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: cpplearner.
aaron.ballman added a comment.

Pulling a comment out of https://reviews.llvm.org/D133853 that's relevant here:

In D133853#3844851 , @cpplearner 
wrote:

> In D133853#3808674 , @aaron.ballman 
> wrote:
>
>> Now I'm wondering why the attribute exists at all. If it's functionally 
>> equivalent to `constexpr` as a keyword, what are the use cases for the 
>> attribute?
>
> It appears that `[[msvc::constexpr]]` does not make a function `constexpr`, 
> but if `[[msvc::constexpr]]` is used in a function definition //and// in a 
> call to that function, then the annotated function call can be evaluated 
> during constant evaluation: https://godbolt.org/z/3MPTsz6Yn
>
> Apparently this is used to implement constexpr `std::construct_at`, which 
> needs to call placement `operator new`, but the latter is not `constexpr`.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt added a comment.

> TODO: I think, I'll need to read more about constexpr for functions in 
> standard (and LLVM code), to add relevant restrictions here.

In the standard I was able to find the following paragraphs about `constexpr`:

1. Neither constructor nor destructor can be constexpr if the class has some 
virtual base class; this holds for `[[msvc::constexpr]]` in MSVC 14.33
2. `constexpr` is not a part of function signature, so same non-constexpr 
function cannot be defined. The same holds for C++11 attributes, and 
`[[msvc::constexpr]]` in MSVC 14.33

I will add two relevant tests in `msvc-attrs-invalid.cpp`

Regarding LLVM code, I made the following observations:

- `FunctionDecl::isConstexprSpecified()` is mostly used for printing code, in 
other words, to check that the function is `constexpr` (and not `consteval`).
- `FunctionDecl::getConstexprKind()` is used by:
- `ASTImporter.cpp` - seems that, it should take care of attributes 
independently from the `getConstexprKind()`
- `ASTWriterDecl.cpp` - seems that, it should take care of attributes 
independently from the `getConstexprKind()`
- `SemaDecl.cpp` / `SemaDeclCXX.cpp` - checks that re-declaration was not 
performed with different constexpr specifier. MSVC and clang with current 
implementation allows re-declaration with different attributes (there are tests 
with `f2-f5` in `msvc-attrs.cpp`)
- `SemaDeclCXX` - enables C++ constexpr constructor inheritance - problem - my 
current implementation supports it, whereas MSVC doesn't:

  // Check '[[msvc::constexpr]]' is not taken into account during constructor 
inheritance
  struct s2 {
  [[msvc::constexpr]] s2() {}
  constexpr operator bool() { return false; };
  };
  
  struct s3 : s2 {
  constexpr operator bool() { return true; };
  };
  static_assert(s3()); // MSVC: C2131: expression did not evaluate to a constant
  static_assert(s3()); // clang with my patch: OK

- `SemaTemplate.cpp` - enables C++ template re-specialization with different 
constexpr specifier, not a problem, because we are allowed to do the same with 
attributes (don't we?)
- `SemaTemplateInstantiateDecl.cpp` (`TemplateDeclInstantiator`) - enables C++ 
template instantiation with same constexpr specifier

  template
  struct s1 {
  [[msvc::constexpr]] s1() {}
  constexpr operator bool() { return true; };
  };
  
  static_assert(s1()); // MSVC: C2131: expression did not evaluate to a 
constant
  static_assert(s1()); // clang with my patch: OK

- `TreeTransform.h` - code is related to lambdas - out-of-scope of the current 
patch.

Regarding `MSVC:C2131` I found one more problem:

  [[msvc::constexpr]] int C() { return 0; }
  constexpr int V = C(); // MSVC: C2131: expression did not evaluate to a 
constant
  constexpr int W = C(); // clang with my patch: OK

I am starting to question my patch - does LLVM really need it, if

1. There's no documentation for `[[msvc::constexpr]]`
2. The trivial implementation makes it an alias, but the semantic behavior is 
different - we could fix that, but there are lots of things to check

@aaron.ballman WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt planned changes to this revision.
RIscRIpt added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4995-4997
+  case ParsedAttr::AT_MSConstexpr:
+D->addAttr(::new (S.Context) MSConstexprAttr(S.Context, AL));
+return;

aaron.ballman wrote:
> This looks incorrect to me -- constexpr isn't a calling convention, so I 
> think this code should be removed.
Oops, removed.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7057-7058
+static void handleMSConstexprAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (auto *FD = dyn_cast(D))
+FD->setConstexprKind(ConstexprSpecKind::Constexpr);
+  D->addAttr(::new (S.Context) MSConstexprAttr(S.Context, AL));

aaron.ballman wrote:
> We can use `cast` here because we know the declaration must have already been 
> determined to be a function (from the subjects list in Attr.td).
> 
> That said, I think there's more semantic checking we want to do here. For 
> example, can you use this attribute on a virtual function? What about a 
> function already marked `constexpr`? Even more scary, what about a function 
> marked `consteval`?
> 
> Also, I presume a `constexpr` function should be able to call an 
> `[[msvc::constexpr]]` function and vice versa?
> We can use cast here because we know the declaration must have already been 
> determined to be a function (from the subjects list in Attr.td).

Indeed. Changed.

> That said, I think there's more semantic checking we want to do here. For 
> example, can you use this attribute on a virtual function? What about a 
> function already marked constexpr? Even more scary, what about a function 
> marked consteval?

Good questions. By running `strings c1xx.dll` I was able to find only the 
following diagnostic messages regarding `[[msvc::constexpr]]`:
```
[[msvc::constexpr]] may only be applied to statements and functions"
[[msvc::constexpr]] cannot be applied to a 'constexpr' or 'consteval' function"
```

I've made a similar check here and added relevant test cases in 
`msvc-attrs-invalid.cpp`. But in order to make it possible, I had to change 
`FunctionDecl::isConstexpr()`, please check new changes.

//TODO//: I think, I'll need to read more about `constexpr` for functions in 
standard (and LLVM code), to add relevant restrictions here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt updated this revision to Diff 464522.
RIscRIpt retitled this revision from "[AST] Add C++11 attribute 
msvc::constexpr" to "Add C++11 attribute msvc::constexpr".
RIscRIpt added a comment.

Add more tests, don't alter constexprKind of `[[msvc::constexpr]]` functions - 
instead change implementation of `isConstexpr`

In D134475#3827712 , @aaron.ballman 
wrote:

> ... we're missing some significant test coverage for it. I had some 
> suggestions for specific things we should be thinking about, but another 
> useful test would be to modify an existing constexpr test to add a RUN line 
> enabling ms extensions, and use a macro to switch between `constexpr` and 
> `[[msvc::constexpr]]` based on those RUN lines. Basically, ensure that 
> `[[msvc::constexpr]]` gives the same behavior (both in terms of evaluation 
> and in terms of semantic checking) as `constexpr`. WDYT?

That's a good idea. Based on my experiments with MSVC, I am more convinced that 
`constexpr` and `[[msvc::constexpr]]` are synonyms (with some extra 
undocumented (yet?) features by MSFT). I added more tests as per your 
suggestion.
Regarding existing tests, I was able to find only 
`clang/test/AST/Interp/functions.cpp` which uses `constexpr` only with 
functions (otherwise `-Dconstexpr=[[msvc::constexpr]]` would break code in case 
there are `constexpr` variables) - I added `RUN` lines there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/Interp/functions.cpp
  clang/test/AST/msvc-attrs-invalid.cpp
  clang/test/AST/msvc-attrs.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -83,6 +83,7 @@
 // CHECK-NEXT: LoaderUninitialized (SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: Lockable (SubjectMatchRule_record)
 // CHECK-NEXT: MIGServerRoutine (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_block)
+// CHECK-NEXT: MSConstexpr (SubjectMatchRule_function)
 // CHECK-NEXT: MSStruct (SubjectMatchRule_record)
 // CHECK-NEXT: MaybeUndef (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: MicroMips (SubjectMatchRule_function)
Index: clang/test/AST/msvc-attrs.cpp
===
--- /dev/null
+++ clang/test/AST/msvc-attrs.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -fms-extensions -std=c++20 -ast-dump %s | FileCheck %s
+// RUN: not %clang_cc1 -Werror=ignored-attributes -ast-dump %s 2> %t.stderr.txt
+// RUN: FileCheck -check-prefix CHECK-DIAG-NO-MSX %s < %t.stderr.txt
+
+// CHECK: msvc-attrs.cpp:[[@LINE+3]]:21, col:32> col:26 f0 'void ()'
+// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} 
+// CHECK-DIAG-NO-MSX: msvc-attrs.cpp:[[@LINE+1]]:3: error: 'constexpr' attribute ignored
+[[msvc::constexpr]] void f0() {}
+
+struct s0 {
+  // CHECK: CXXConstructorDecl 0x{{[0-9a-f]+}}  col:23 s0 'void ()' implicit-inline
+  // CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} 
+  [[msvc::constexpr]] s0() {}
+
+  // CHECK: CXXDestructorDecl 0x{{[0-9a-f]+}}  col:23 ~s0 'void () noexcept' implicit-inline
+  // CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} 
+  [[msvc::constexpr]] ~s0() {}
+
+  // CHECK: CXXMethodDecl 0x{{[0-9a-f]+}}  col:36 used f1 'void ()' virtual implicit-inline
+  // CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} 
+  [[msvc::constexpr]] virtual void f1() {}
+};
+
+// Check 'constexpr' and '[[msvc::constexpr]]' functions can call each other
+void f2();
+constexpr void f3();
+
+[[msvc::constexpr]] void f2() { f3(); }
+constexpr void f3() { f2(); }
+
+[[msvc::constexpr]] void f4();
+constexpr void f5();
+
+void f4() { f5(); }
+constexpr void f5() { f4(); }
Index: clang/test/AST/msvc-attrs-invalid.cpp
===
--- /dev/null
+++ clang/test/AST/msvc-attrs-invalid.cpp
@@ -0,0 +1,14 @@
+// RUN: not %clang_cc1 -fms-extensions -std=c++20 -ast-dump %s 2> %t.stderr.txt
+// RUN: FileCheck %s < %t.stderr.txt
+
+void runtime() {}
+
+// CHECK: msvc-attrs-invalid.cpp:[[@LINE+1]]:26: error: constexpr function never produces a constant expression [-Winvalid-constexpr]
+[[msvc::constexpr]] void f0() { runtime(); }
+
+// CHECK: msvc-attrs-invalid.cpp:[[@LINE+1]]:3: error: {{\[\[}}msvc::constexpr{{]]}} cannot be applied to a 'constexpr' or 'consteval' function 'f1'
+[[msvc::constexpr]] constexpr void f1() {}
+
+// CHECK: msvc-attrs-invalid.cpp:[[@LINE+1]]:3: error: {{\[\[}}msvc::constexpr{{]]}