mboehme marked 3 inline comments as done. mboehme added a comment. In D111548#3483275 <https://reviews.llvm.org/D111548#3483275>, @erichkeane wrote:
> I don't really know how useful this ends up being, these attributes (since > they are part of `AttributedType` end up disappearing pretty quickly/easily. > Anything that causes canonicalization will cause these to go away, This is true, but I also think we don't have much choice in the matter if we don't want these attributes to affect the C++ type system. We discuss this at length in the RFC: https://discourse.llvm.org/t/rfc-new-attribute-annotate-type-iteration-2/61378#rationale-5 I go into more depth in this comment: https://discourse.llvm.org/t/rfc-new-attribute-annotate-type-iteration-2/61378/4?u=martinboehme However, it is possible for a static analysis to propagate the annotations through typedefs, template arguments and such, and we do so in the lifetime static analysis (see also below). > they don't apply to references to these types, etc. Not sure what you mean here -- do you mean that the annotations don't propagate through typedefs? As noted, it's possible for a static analysis tool to perform this propagation, and the discussion here is again relevant: https://discourse.llvm.org/t/rfc-new-attribute-annotate-type-iteration-2/61378/4?u=martinboehme > But I otherwise don't have concerns. > > HOWEVER, have you implemented the lifetime static analysis that you're > wanting to do on top of this already Yes, we have. We are able to propagate lifetime annotations through non-trivial C++ constructs, including template arguments. Based on what our current prototype can do, we're confident these annotations will do everything we need them to. We also believe they are general enough to be useful for other types of static analysis. ================ Comment at: clang/lib/AST/TypePrinter.cpp:1689 + // the type. + OS << " [[clang::annotate_type]]"; + return; ---------------- erichkeane wrote: > My opinion (though not attached to it) would be `clang::annotate_type(...)` > or, `clang::annotate_type(<unavailable>)` or something like that. Good idea -- this makes it more obvious that the arguments were omitted. Changed! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111548/new/ https://reviews.llvm.org/D111548 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
