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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to