[PATCH] D92742: [clang] Add support for attribute 'swift_async'

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 7 06:33:26 PST 2020


aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM aside from a minor nit.



================
Comment at: clang/include/clang/Basic/AttrDocs.td:4367
+``actuallyAsync:callThisAsync`` wouldn't have been imported as ``async`` if not
+for ``swift_async`` because it doesn't match the naming convention.
+
----------------
for `swift_async` because -> for the `swift_async` attribute because


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6046
+  if (!SwiftAsyncAttr::ConvertStrToKind(II->getName(), Kind)) {
+    S.Diag(AL.getLoc(), diag::err_swift_async_no_access) << AL << II;
+    return;
----------------
FWIW, we usually use `warn_attribute_type_not_supported` for this diagnostic rather than coming up with a unique diagnostic per enumeration. (This sort of feels like something we should use tablegen to handle anyway.)

I don't insist on a change as your diagnostic is slightly better than what we usually use, but if you feel like making the change for consistency with other attribute behavior, that's fine by me as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92742/new/

https://reviews.llvm.org/D92742



More information about the cfe-commits mailing list