aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a comment.
This is missing all the usual Sema tests that the attribute only applies to
functions, takes no arguments, only works in web assembly targets, etc. Also,
the changes should come with a release note so users know about the new
attribute.
================
Comment at: clang/include/clang/Basic/AttrDocs.td:5608-5612
+Clang supports the ``__attribute__((wasm_async))``
+attribute for the WebAssembly target. This attribute may be attached to a
+function definition, which indicates the function will be used with JavaScript
+promise integration (JSPI). The attribute will cause the creation of a custom
+section named "async" that contains each wasm_async function's index value.
----------------
This could be my ignorance of web assembly showing, but the docs don't really
help me understand when you'd want to use this attribute. Perhaps a link to
what JSPI is and a code example would help a little bit? Or is this more of a
low-level implementation detail kind of attribute where folks already know the
domain?
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:898
+ if (FD->getAttr<WebAssemblyAsyncAttr>()) {
+ llvm::Function *Fn = cast<llvm::Function>(GV);
+ llvm::AttrBuilder B(GV->getContext());
----------------
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7637-7640
+ if (!isFunctionOrMethod(D)) {
+ S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+ << "'wasm_async'" << ExpectedFunction;
+ return;
----------------
This should be handled automatically for you by the common attribute handling
code, so I think you can remove this.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7645
+ if (FD->isThisDeclarationADefinition()) {
+ S.Diag(D->getLocation(), diag::err_alias_is_definition) << FD << 0;
+ return;
----------------
This diagnostic doesn't make sense to me -- how does this attribute relate to
ifuncs or aliases? Why should users be prohibited from writing the attribute on
a definition?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150803/new/
https://reviews.llvm.org/D150803
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
