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<FunctionDecl>(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

Reply via email to