[PATCH] D63954: Add lifetime categories attributes

Dmitri Gribenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 1 10:51:52 PDT 2019


gribozavr added a comment.

> I agree. In a follow-up patch we will add the attributes to STL types during parsing. Do you think it is OK to always add the attributes or should we only do it if a flag, e.g.  -wlifetime is added to the compiler invocation?

Warning flags should not change the AST -- compiler engineers don't expect that. So if Clang is going to perform inference, it should either always do it, or it should be guarded by an `-f` flag, not a `-W` flag.

> On the other hand I still think it is useful to give the users the option to maintain a header with forward declarations to add the annotations to other (non-standard) 3rd party types. These headers might be fragile to 3rd party changes but could still be a better option than maintaining patches on top of 3rd party libraries. Having API notes upstream would be a superior solution here and I might look into upstreaming it at some point, but I think this is something that could be addressed later on. What do you think?

I think it is acceptable for 3rd party libraries, but there's already a solution for 3rd party libraries -- API notes in Swift's fork of Clang. It has been successfully used by Apple for 5 years for this exact purpose (adding attributes to existing libraries without changing headers), and only needs to be upstreamed to Clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954





More information about the cfe-commits mailing list