[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4724 + zx_status_t zx_socket_create(uint32_t options, + zx_handle_t __attribute__((acquire_handle)) * out0, + zx_handle_t* out1

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

paulherman added inline comments. Herald added subscribers: manas, steakhal, jdoerfert, ASDenysPetrov, phosek. Comment at: clang/include/clang/Basic/AttrDocs.td:4724 + zx_status_t zx_socket_create(uint32_t options, + zx_handle_t

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun marked an inline comment as done. xazax.hun added a comment. Thanks for the review! :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 ___ cfe-commits mailing list

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

This revision was automatically updated to reflect the committed changes. Closed by commit rGfe17b30a7957: [attributes][analyzer] Add annotations for handles. (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D70469?vs=234341=234940#toc Repository: rG LLVM Github

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with the nits fixed, thank you! Comment at: clang/include/clang/Basic/Attr.td:3475 + +def AcquireHandle : DeclOrTypeAttr { + let Spellings =

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3475 + +def AcquireHandle : DeclOrTypeAttr { + let Spellings = [Clang<"acquire_handle">]; I have no idea whether this is a problem or not (and I'm not certain if you know

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun updated this revision to Diff 234341. xazax.hun added a comment. - Added string argument to specify the handle type. - Only AcquireHandleAttr can be a type attribute and it can only be applied to function types. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7424 + ParsedAttr ) { + if (CurType->isFunctionType()) { +State.getSema().Diag(Attr.getLoc(),

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman added a comment. In D70469#1779906 , @xazax.hun wrote: > In D70469#1778609 , @NoQ wrote: > > > What about `__attribute__((acquire_handle("fuchsia")))` > > > I would by fine with this as well. Aaron?

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

NoQ added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7424 + ParsedAttr ) { + if (CurType->isFunctionType()) { +State.getSema().Diag(Attr.getLoc(), diag::warn_type_attribute_wrong_type) xazax.hun wrote: >

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7424 + ParsedAttr ) { + if (CurType->isFunctionType()) { +State.getSema().Diag(Attr.getLoc(),

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun added a comment. In D70469#1778609 , @NoQ wrote: > What about `__attribute__((acquire_handle("fuchsia")))` I would by fine with this as well. Aaron? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

NoQ added a comment. What about `__attribute__((acquire_handle("fuchsia")))`? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun added a comment. In D70469#1777858 , @aaron.ballman wrote: > In D70469#1776523 , @NoQ wrote: > > > It still mildly worries me that the attributes aren't truly reusable, as > > the exact meaning of the

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman added a comment. In D70469#1776523 , @NoQ wrote: > It still mildly worries me that the attributes aren't truly reusable, as the > exact meaning of the attribute depends on the domain. In particular, every > domain is expected to have

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

NoQ added a comment. It still mildly worries me that the attributes aren't truly reusable, as the exact meaning of the attribute depends on the domain. In particular, every domain is expected to have different approaches to error handling, i.e. a function that creates or destroys a handle may

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7424 + ParsedAttr ) { + if (CurType->isFunctionType()) { +State.getSema().Diag(Attr.getLoc(), diag::warn_type_attribute_wrong_type) xazax.hun wrote: >

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun updated this revision to Diff 232873. xazax.hun marked 2 inline comments as done. xazax.hun added a comment. - Make sure typedefs are supported. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 Files:

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7424 + ParsedAttr ) { + if (CurType->isFunctionType()) { +State.getSema().Diag(Attr.getLoc(),

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7424 + ParsedAttr ) { + if (CurType->isFunctionType()) { +State.getSema().Diag(Attr.getLoc(),

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun updated this revision to Diff 232723. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun marked 2 inline comments as done. xazax.hun added inline comments. Comment at: clang/test/Sema/attr-handles.cpp:20-21 +void ht(int __attribute__((acquire_handle(1))) *a); // expected-error {{'acquire_handle' attribute takes no arguments}} +int it()

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7424 + ParsedAttr ) { + if (CurType->isFunctionType()) { +State.getSema().Diag(Attr.getLoc(), diag::warn_type_attribute_wrong_type) Is this correct, or

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun updated this revision to Diff 232681. xazax.hun marked 7 inline comments as done. xazax.hun added a comment. - Make the annotation acceptable on both types and declarations. - Fix some TODOs. - Teach TypePrinter to print the annotations. - Address review comments. CHANGES SINCE LAST

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7412 + diag::warn_type_attribute_wrong_type_str) +<< Attr.getAttrName()->getName() << "non-function" << CurType; +return; aaron.ballman wrote: > You

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman added a comment. In D70469#1766084 , @xazax.hun wrote: > @aaron.ballman > > Type attributes are definitely more flexible as in they can be applied in > more contexts. These attributes, however, make no sense outside of a function >

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun added a comment. In the meantime, I realized lambdas are actually not that important. Lambdas are only usable in C++, and in C++ one should prefer to use move only RAII wrapper for handles. This reduces the problem into a use after move and we already have a check for that. These

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun added a comment. @aaron.ballman Type attributes are definitely more flexible as in they can be applied in more contexts. These attributes, however, make no sense outside of a function declaration, or maybe a type alias. So I feel like we could argue on both sides. I made a minimal

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun updated this revision to Diff 231784. xazax.hun added a comment. - Convert to type attributes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3445 + let Spellings = [Clang<"acquire_handle">]; + let Subjects = SubjectList<[Function, ParmVar], ErrorDiag>; + let Documentation = [AcquireHandleDocs]; xazax.hun wrote: >

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3445 + let Spellings = [Clang<"acquire_handle">]; + let Subjects = SubjectList<[Function, ParmVar], ErrorDiag>; + let Documentation = [AcquireHandleDocs]; aaron.ballman wrote: >

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun updated this revision to Diff 231307. xazax.hun marked 5 inline comments as done. xazax.hun added a comment. - Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 Files: clang/include/clang/Basic/Attr.td

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6524 + if (auto *PVD = dyn_cast(D)) { +if (PVD->getType()->isIntegerType()) { + S.Diag(AL.getLoc(), diag::err_attribute_output_parameter)

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3445 + let Spellings = [Clang<"acquire_handle">]; + let Subjects = SubjectList<[Function, ParmVar], ErrorDiag>; + let Documentation = [AcquireHandleDocs]; xazax.hun wrote: >

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun updated this revision to Diff 230347. xazax.hun marked 8 inline comments as done. xazax.hun added a comment. - Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 Files: clang/include/clang/Basic/Attr.td

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3450 +def UseHandle : InheritableParamAttr { + let Spellings = [Clang<"use_handle">]; + let Documentation = [UseHandleDocs]; xazax.hun wrote: > aaron.ballman wrote: > > Should

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

aaron.ballman added a comment. This is also missing tests for all the Sema handling (that the attributes only appertain to the specified subjects, accept no arguments, etc). Comment at: clang/include/clang/Basic/Attr.td:3445 + let Spellings = [Clang<"acquire_handle">]; +

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun updated this revision to Diff 230294. xazax.hun added a comment. - Check trivial cases when the acquire_handle attribute is not on an output parameter. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 Files:

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun marked 3 inline comments as done. xazax.hun added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3445 + let Spellings = [Clang<"acquire_handle">]; + let Subjects = SubjectList<[Function, ParmVar], ErrorDiag>; + let Documentation =

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7287 + case ParsedAttr::AT_AcquireHandle: +handleSimpleAttribute(S, D, AL); +break; NoQ wrote: > xazax.hun wrote: > > NoQ wrote: > >

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

NoQ added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7287 + case ParsedAttr::AT_AcquireHandle: +handleSimpleAttribute(S, D, AL); +break; xazax.hun wrote: > NoQ wrote: > > The next obvious step here would be to type-check the

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7287 + case ParsedAttr::AT_AcquireHandle: +handleSimpleAttribute(S, D, AL); +break; NoQ wrote: > The next obvious step here would be

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

NoQ added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7287 + case ParsedAttr::AT_AcquireHandle: +handleSimpleAttribute(S, D, AL); +break; The next obvious step here would be to type-check the declaration to make sure that it's actually

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

xazax.hun created this revision. xazax.hun added reviewers: aaron.ballman, NoQ. xazax.hun added a project: clang. Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. These are three new