aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a comment.
Thank you for this! I think it's heading in the right direction, but 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?
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4995-4997
+ case ParsedAttr::AT_MSConstexpr:
+ D->addAttr(::new (S.Context) MSConstexprAttr(S.Context, AL));
+ return;
----------------
This looks incorrect to me -- constexpr isn't a calling convention, so I think
this code should be 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));
----------------
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?
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
