[PATCH] D111548: [Clang] Add the `annotate_type` attribute

[Martin Böhme via Phabricator via cfe-commits]

mboehme marked 2 inline comments as done.
mboehme added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:6387
+additional information specific to the annotation category. The optional
+arguments must be constant expressions of arbitrary type.
+
----------------
xazax.hun wrote:
> Do we want to mention that it is not actually part of the type system (i.e., annotated and unannotated types are considered the same)?
Good point -- I've added some relevant parts from the RFC below.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:6393
+
+  int* [[clang::annotate("category1", "foo", 1)]] f(int[[clang::annotate("category2"]] *);
+
----------------
xazax.hun wrote:
> Missing closing paren for the second annotate. 
Oops -- thanks for catching. Fixed.


================
Comment at: clang/lib/AST/TypePrinter.cpp:1686
+  // would require retrieving the attribute arguments, which we don't have here.
+  if (T->getAttrKind() == attr::AnnotateType)
+    return;
----------------
xazax.hun wrote:
> Would it make sense to print something without the arguments? I wonder which behavior would be less confusing.
TBH I'm not sure. Is TypePrinter used only to produce debug output or is it required that the output of TypePrinter will parse again correctly?

The reason I'm asking is because `annotate_type` has a mandatory first argument; if we need the output to be parseable, we would have to print some dummy string, e.g. `[[clang::annotate_type("arguments_omitted")]]`. This seems a bit strange, but maybe it's still worth doing. OTOH, if the output is used only for debugging, I guess we could print something like `[[clang::annotate_type(...)]]`, which doesn't parse but by that very nature makes it clear that we don't have all the information here.

I guess both of these options could have limited use -- they would at least make it clear that there is an annotation on the type, though without the arguments, we can't know what the actual annotation is.

Would appreciate some more input on the wider context here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111548