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

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/Basic/AttrDocs.td:3780
+represent an implicit "this" pointer in class methods. If there is no implicit
+"this" pointer it shall not be referenced. The index '-1', or the name "?",
+represents an unknown callback callee argument. This can be a value which is
----------------
The `'?'` here needs to change to be `'__'` instead.


================
Comment at: include/clang/Basic/Builtins.def:96
 //  V:N: -> requires vectors of at least N bits to be legal
+//  C<N,M_0,...,M_k> -> callback behavior: argument N is called with argument M_0, ..., M_k as payload
 //  FIXME: gcc has nonnull
----------------
Wrap to 80-col


================
Comment at: lib/Sema/SemaDeclAttr.cpp:3483
+
+  NameIdxMapping["__this"] = 0;
+
----------------
jdoerfert wrote:
> aaron.ballman wrote:
> > This doesn't match the documentation either, but I'm less clear on why the double underscores are used.
> If you use `this`, the lexer will generate the special "this" token. That one is checked explicitly to be only used inside of non-static class methods. If you have an idea how to avoid this check or make it consider uses in the attribute as OK, please let me know.
Why do you want to avoid that check? It seems like that's a very good check to have -- what circumstances would you expect a user to use `this` in a something other than a non-static member function?


================
Comment at: lib/Sema/SemaDeclAttr.cpp:3492
+  SmallVector<int, 8> EncodingIndices;
+  for (unsigned u = 0, e = AL.getNumArgs(); u < e; u++) {
+
----------------
jdoerfert wrote:
> aaron.ballman wrote:
> > Identifiers don't match the usual naming conventions.  Prefer `++U` as well.
> OK.
> 
> 
> > Prefer ++U as well.
> 
> Out of curiosity, why?
Because the previous value of `U` is not needed (you're executing the instruction only for its side effects, not its value).


Repository:
  rC Clang

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

https://reviews.llvm.org/D55483