[PATCH] D63954: Add lifetime categories attributes

mgehre added a comment. I will include your latest comments into D64448 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 ___ cfe-commits maili

[PATCH] D63954: Add lifetime categories attributes

gribozavr added inline comments. Comment at: cfe/trunk/include/clang/Basic/AttrDocs.td:4252 + +The argument ``T`` is optional and currently ignored. +This attribute may be used by analysis tools and has no effect on code "and is currently ignored" even better:

[PATCH] D63954: Add lifetime categories attributes

mgehre added a comment. Thank you very much for dedicating your time for the review. It improved the code a lot! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 ___ cfe-commit

[PATCH] D63954: Add lifetime categories attributes

This revision was automatically updated to reflect the committed changes. Closed by commit rL367040: Add lifetime categories attributes (authored by mgehre, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a minor formatting nit. Thank you for working on these attributes! Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2517 +def err_a

[PATCH] D63954: Add lifetime categories attributes

mgehre marked 9 inline comments as done. mgehre added inline comments. Comment at: clang/test/SemaCXX/attr-gsl-owner-pointer-parsing.cpp:2 +// RUN: %clang_cc1 -fsyntax-only -verify %s + +int [[gsl::Owner]] i; aaron.ballman wrote: > This file tests part of parsing

[PATCH] D63954: Add lifetime categories attributes

mgehre updated this revision to Diff 211600. mgehre marked an inline comment as done. mgehre added a comment. - Fix all comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 Files: clang/include/clang/

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2771 + let Spellings = [CXX11<"gsl", "Owner">]; + let Subjects = SubjectList<[CXXRecord]>; + let Args = [TypeArgument<"DerefType", /*opt=*/1>]; This subject should be `Struct` a

[PATCH] D63954: Add lifetime categories attributes

mgehre updated this revision to Diff 211370. mgehre marked 4 inline comments as done. mgehre added a comment. - Diagnose unions; improve other diagnostics; fix all comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.ll

[PATCH] D63954: Add lifetime categories attributes

mgehre marked 2 inline comments as done. mgehre added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4167 + +The attribute ``[[gsl::Owner(T)]]`` applies to structs and classes that own an +object of type ``T``: aaron.ballman wrote: > Do either

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4167 + +The attribute ``[[gsl::Owner(T)]]`` applies to structs and classes that own an +object of type ``T``: Do either of these attributes make sense on a union type? If so, m

[PATCH] D63954: Add lifetime categories attributes

mgehre updated this revision to Diff 211008. mgehre marked 6 inline comments as done. mgehre added a comment. - Disallow reference and array types as DerefType - Add example to documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/

[PATCH] D63954: Add lifetime categories attributes

mgehre added a comment. I think that I have addressed all open comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4164 + let Content = [{ +When annotating a class ``O`` with ``[[gsl::Owner(T)]]``, then each function +that returns cv-qualified ``T&`` or ``T*`` is assum

[PATCH] D63954: Add lifetime categories attributes

gribozavr added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4164 + let Content = [{ +When annotating a class ``O`` with ``[[gsl::Owner(T)]]``, then each function +that returns cv-qualified ``T&`` or ``T*`` is assumed to return a mgehre wro

[PATCH] D63954: Add lifetime categories attributes

mgehre updated this revision to Diff 209750. mgehre marked 23 inline comments as done. mgehre added a comment. - Don't crash when pretty-printing an Attribute with optional type argument - Don't crash AST dump when Owner/Pointer has no Type argument - gsl::Owner/gsl::Pointer: Make DerefType parame

[PATCH] D63954: Add lifetime categories attributes

mgehre 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

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2770 +def Owner : InheritableAttr { + let Spellings = [CXX11<"gsl", "Owner">]; + let Subjects = SubjectList<[CXXRecord]>; xazax.hun wrote: > aaron.ballman wrote: > > xazax.hun wr

[PATCH] D63954: Add lifetime categories attributes

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 wr

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2770 +def Owner : InheritableAttr { + let Spellings = [CXX11<"gsl", "Owner">]; + let Subjects = SubjectList<[CXXRecord]>; xazax.hun wrote: > aaron.ballman wrote: > > Has Microsof

[PATCH] D63954: Add lifetime categories attributes

gribozavr added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4164 + let Content = [{ +When annotating a class ``O`` with ``[[gsl::Owner(T)]]``, then each function +that returns cv-qualified ``T&`` or ``T*`` is assumed to return a Slightly b

[PATCH] D63954: Add lifetime categories attributes

xazax.hun added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4177 + let Content = [{ +When annotating a class ``P`` with ``[[gsl::Pointer(T)]]``, it assumed to be a +non-owning type whose objects can point to ``T`` type objects or dangle. a

[PATCH] D63954: Add lifetime categories attributes

mgehre updated this revision to Diff 207613. mgehre added a comment. - Make the list of attribute properties more consistent. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 Files: clang/include/clang/Bas

[PATCH] D63954: Add lifetime categories attributes

mgehre updated this revision to Diff 207400. mgehre added a comment. - Add newline at end of file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 Files: clang/include/clang/Basic/Attr.td clang/include/c

[PATCH] D63954: Add lifetime categories attributes

mgehre added a comment. I understand that putting types on forward declared header (standard libraries / third party libraries) is not a approach we should favor. Thank you for your good arguments. API notes seems like a really good approach for this (I would like to help with the upstreaming),

[PATCH] D63954: Add lifetime categories attributes

gribozavr added a comment. In D63954#1565128 , @xazax.hun wrote: > In D63954#1565109 , @gribozavr wrote: > > > > I agree. In a follow-up patch we will add the attributes to STL types > > > during parsing. Do you th

[PATCH] D63954: Add lifetime categories attributes

mgehre updated this revision to Diff 207381. mgehre marked 5 inline comments as done. mgehre added a comment. - Address more review comments. - git-clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D6395

[PATCH] D63954: Add lifetime categories attributes

xazax.hun added a comment. In D63954#1565109 , @gribozavr wrote: > > 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.

[PATCH] D63954: Add lifetime categories attributes

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

[PATCH] D63954: Add lifetime categories attributes

xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4553 + // we always add (and check) the attribute to the cannonical decl. + D = D->getCanonicalDecl(); + if(AL.getKind() == ParsedAttr::AT_Owner) { aaron.ballman wrote: > xazax.hun w

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4553 + // we always add (and check) the attribute to the cannonical decl. + D = D->getCanonicalDecl(); + if(AL.getKind() == ParsedAttr::AT_Owner) { xazax.hun wrote: > aaron.ballm

[PATCH] D63954: Add lifetime categories attributes

xazax.hun added a comment. In D63954#1564448 , @gribozavr wrote: > > We explicitly allow to add an annotation after the definition of the class > > to allow adding annotations to external source from by the user > > Asking users to forward-declare anythin

[PATCH] D63954: Add lifetime categories attributes

gribozavr added a comment. > We explicitly allow to add an annotation after the definition of the class to > allow adding annotations to external source from by the user Asking users to forward-declare anything from the standard library is a very bad idea -- in fact it is undefined behavior. h

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman added a comment. In D63954#1562928 , @erik.pilkington wrote: > > We explicitly allow to add an annotation after > > the definition of the class to allow adding annotations > > to external source from by the user, e.g. > > > > #include

[PATCH] D63954: Add lifetime categories attributes

mgehre marked 2 inline comments as done. mgehre added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4560-4561 + if(AL.getKind() == ParsedAttr::AT_Owner) { +if (checkAttrMutualExclusion(S, D, AL)) + return; +if (const auto *Attr = D->getAttr()) {

[PATCH] D63954: Add lifetime categories attributes

mgehre updated this revision to Diff 207235. mgehre added a comment. Address some review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 Files: clang/include/clang/Basic/Attr.td clang/include/

[PATCH] D63954: Add lifetime categories attributes

mgehre added a comment. We can and should explore both options and see what works best. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 ___ cfe-commits mailing

[PATCH] D63954: Add lifetime categories attributes

xazax.hun added a comment. I actually think we should hard code STL (and maybe some other popular) types into the compiler. I.e.: if an STL type is unannotated and the warnings are turned on, we could look up the default annotations from a table and append them during parsing. This could be don

[PATCH] D63954: Add lifetime categories attributes

mgehre added a comment. We can try to add the annotations to libcxx (which will still take time), but there are also other standard libraries (libstdc++, MSVC) that can be used with clang. Eventually everybody will have lifetime annotations (of course ;-) ), but in the meantime there seems to be n

[PATCH] D63954: Add lifetime categories attributes

erik.pilkington added a comment. > We explicitly allow to add an annotation after > the definition of the class to allow adding annotations > to external source from by the user, e.g. > > #include > > namespace std { > template > class [[gsl::Owner(T)]] vector; > } Wait, does tha

[PATCH] D63954: Add lifetime categories attributes

mgehre updated this revision to Diff 207138. mgehre added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63954/new/ https://reviews.llvm.org/D63954 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.

[PATCH] D63954: Add lifetime categories attributes

mgehre created this revision. mgehre added a reviewer: gribozavr. Herald added a project: clang. This is the first part of work announced in "[RFC] Adding lifetime analysis to clang" [0], i.e. the addition of the [[gsl::Owner(T)]] and [[gsl::Pointer(T)]] attributes, which will enable user-defined