[PATCH] D55483: Introduce the callback attribute and emit !callback metadata

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 15 12:53:12 PST 2019


aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from some minor nits.

However, please hold off on committing this until *after* the 8.0 branch happens. While I don't expect there to be issues with this patch, it does introduce a pretty novel new way to interact with attributes and I want to ensure that the community has the opportunity to react to any major issues with this approach, and I'm not certain a month or so will be enough time for that. If you need this patch to go in *before* the branch for some reason, please mention it! I'm not strictly opposed to it going in sooner rather than later, but if there's not a pressing need for the functionality, I'd prefer to wait out of an abundance of caution.



================
Comment at: include/clang/Basic/AttrDocs.td:3778
+The callback callee is required to be callable with the number, and order, of
+the specified arguments. The index `0`, or the identifier `__this`, is used to
+represent an implicit "this" pointer in class methods. If there is no implicit
----------------
`__this` should be `this` now.


================
Comment at: utils/TableGen/ClangAttrEmitter.cpp:2169
 
+static bool keywordThisIsaIdentifierInArgument(Record *Arg) {
+  return !Arg->getSuperClasses().empty() &&
----------------
`const Record *`


================
Comment at: utils/TableGen/ClangAttrEmitter.cpp:2177-2178
+
+static void emitClangAttrThisIsaIdentifierArgList(RecordKeeper &Records,
+                                                          raw_ostream &OS) {
+  OS << "#if defined(CLANG_ATTR_THIS_ISA_IDENTIFIER_ARG_LIST)\n";
----------------
The formatting looks off here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55483





More information about the llvm-commits mailing list