[PATCH] D63954: Add lifetime categories attributes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:2770
+def Owner : InheritableAttr {
+  let Spellings = [CXX11<"gsl", "Owner">];
+  let Subjects = SubjectList<[CXXRecord]>;
----------------
aaron.ballman wrote:
> xazax.hun wrote:
> > aaron.ballman wrote:
> > > Has Microsoft already added support for this? We had an unfortunate problem with `gsl::suppress` where we introduced the attribute first and MSVC introduced it under a different, incompatible syntax. I would strongly like to avoid repeating that.
> > I will double check this. Her Sutter's paper have this spelling and I believe that the Microsoft implementation is following the same spelling. The main difference is that the MSVC version does not have a type argument at this point but they do plan to add it as an optional argument. (They want to infer the pointee type when it is not provided.) The clang community do not want the inference, hence we made the type argument required. Always providing the type argument should be compatible with future versions of the MSVC implementation.
> I take this to mean that we're not implementing the same attribute as MSVC is, and that worries me.
> 
> > Always providing the type argument should be compatible with future versions of the MSVC implementation.
> 
> The problem is that code written for MSVC (where the type argument is not required) will not compile with Clang (where the type argument is required), if I understand properly.
> 
> Generally speaking, when we implement an attribute from another vendor namespace, we expect the attribute to have the same semantics as whoever "owns" that vendor namespace. It's a bit trickier here because `gsl` isn't really a vendor so much as a collective, so I don't know who is the authority to turn to.
> 
> On a completely different note: would this attribute also make sense within C with a C2x spelling?
I see, that is a valid concern. A followup patch makes the type argument optional so it will be fully compatible with MSVC.  I don't think it makes sense with C or at least I am not sure what would I annotate using these attributes in C code as the subjects are classes. I know that it is possible to simulate classes in C but I am not sure how well the analysis would work in such cases. 

Are you OK with making the argument optional in a followup patch or do you want us to cherry-pick that modification into this one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954