[PATCH] D150803: [WebAssembly] Add a new `wasm_async` clang attribute for marking async functions.

[Aaron Ballman via Phabricator via cfe-commits]

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