[PATCH] D98134: [RFC][POC] Introduce callback argument encoding mode into callback metadata

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:432
 If a statement is marked ``nomerge`` and contains call expressions, those call
-expressions inside the statement will not be merged during optimization. This 
+expressions inside the statement will not be merged during optimization. This
 attribute can be used to prevent the optimizer from obscuring the source
----------------
It looks like a bunch of unrelated whitespace changes snuck in.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:5221
+The first position in the attribute identifies the callback argument encoding
+mode, ``0`` for flat mode, and ``1`` for stacked mode. The following example
+shows the difference between the two modes:
----------------
Rather than use `0` and `1` directly, any reason not to use a named enumerator so that the user can write: `__attribute__((callback(stacked, 3, 4)))`?

Also, should this attribute argument be the last argument in the list and made optional so that existing code will continue to work?


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2940
+def err_callback_attribute_no_encoding_mode : Error<
+  "'callback' attribute specifies no parameter encoding mode">;
 def err_callback_attribute_invalid_callee : Error<
----------------
I'm not certain it's reasonable to turn this into an error given that this attribute already exists in the wild. This basically breaks all users of the attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98134