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