This change is intended as initial setup. The plan is to add
more semantic checks later. I plan to update the documentation
as more semantic checks are added (instead of documenting the
details up front). Most of the code closely mirrors that for
the Swift calling convention. Three places are marked as
[FIXME: swiftasynccc]; those will be addressed once the
corresponding convention is introduced in LLVM.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/include/clang/Basic/AttrDocs.td | ||
---|---|---|
4547 | I have left this as a TODO for now, so that it can be filled in later when the exact details of the convention are implemented in LLVM. |
I don't get how you stack your change on top of someone else's change (the diff seems to have been created with only one commit), but this depends on https://reviews.llvm.org/D95228, which depends on https://reviews.llvm.org/D95044.
You should be able to click the "Edit Related Revisions" link in the top right, click "Parent", and search for / select "D95228".
Broadly LGTM
clang/include/clang/Basic/AttrDocs.td | ||
---|---|---|
4547 | We don't generally put precise CC details in this kind of documentation; the goal is to describe the calling convention at a high level. Here, you should describe it as being compatible with the low-level conventions of Swift async functions, provided it declares the right formal arguments. You should also talk about the special-case treatment of tail calls, especially the extension which adds return (void) to all language modes. | |
clang/lib/AST/MicrosoftMangle.cpp | ||
2754 | Please use consistent formatting with the other cases, here and elsewhere. Keeping the switch compact is better for readability than following an abstract style rule. |
clang/lib/Sema/SemaDeclAttr.cpp | ||
---|---|---|
8110–8119 | There seems to be some weirdness here in handleCallConvAttr where the codepath to attach the attribute to the decl isn't taken. Instead in my examples, it exits out at this line: if (hasDeclarator(D)) return; with an isa<DeclaratorDecl> check. |
clang/lib/AST/MicrosoftMangle.cpp | ||
---|---|---|
2754 | I'm pretty sure I wrote it the right way originally, but saying yes to the clang-tidy prompt by arc broke it. :sigh: |
clang/lib/Sema/SemaDeclAttr.cpp | ||
---|---|---|
8110–8119 | It doesn't need to be attached to the decl; it should be reflected in the type, the same as the other calling conventions. You would need it on the decl for something like an ObjC method. |
clang/include/clang/Basic/AttrDocs.td | ||
---|---|---|
4558 | I don't think the "must be in tail position" here is true. Allow me to suggest an alternative way of saying this:
|
clang/test/CodeGen/debug-info-cc.c | ||
---|---|---|
60–66 | This needs to be changed in some way, it triggers a test failure in its current state. https://github.com/apple/llvm-project/pull/2444 |
- Remove semantic restriction of swift_async_context being only applicable to swiftasynccall.
- Update target checks to make sure we only allow supported targets.
- Update doc comment.
LGTM other than the TargetInfo thing.
clang/include/clang/Basic/TargetInfo.h | ||
---|---|---|
1424 ↗ | (On Diff #324389) | As I commented in the other patch, I think this would be cleaner in the long term if you just implemented it directly in the various target subclasses. |
Permit swift_context and swift_indirect_result parameters for
swiftasynccall functions. (Earlier, these were restricted to
swiftcall-only.)
Combined D95984 into this patch since all necessary LLVM patches have landed, so having the patches separated out is not helpful. Leaving this up for a few days in case there are any final comments before landing.
I have left this as a TODO for now, so that it can be filled in later when the exact details of the convention are implemented in LLVM.