aaron.ballman marked an inline comment as done. aaron.ballman added inline comments.
================ Comment at: clang/lib/Sema/SemaType.cpp:7424 + ParsedAttr &Attr) { + if (CurType->isFunctionType()) { + State.getSema().Diag(Attr.getLoc(), diag::warn_type_attribute_wrong_type) ---------------- aaron.ballman wrote: > Is this correct, or should it be `if (!CurType->isFunctionType())`? I see now that this is only incorrect for the acquire attribute, but not the other two. ================ 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() __attribute__((release_handle)); // expected-warning {{'release_handle' only applies to non-function types; type here is 'int ()'}} +int jt() __attribute__((use_handle)); // expected-warning {{'use_handle' only applies to non-function types; type here is 'int ()'}} +int __attribute__((acquire_handle)) at; // expected-warning {{'acquire_handle' attribute only applies to functions, function pointers, and parameters}} ---------------- xazax.hun wrote: > aaron.ballman wrote: > > These diagnostics look incorrect to me -- I thought we wanted the attribute > > to apply to function types? > > > > You should also add some test cases where the attribute is applied to a > > function pointer type. > I think this one can be a bit confusing. We can apply this to the types of > the parameter or the return type. I thought restricting to users to put it > either on the parameter or the return type would make it more obvious what is > the target of this attribute. In case this is not idiomatic, I can let the > user to attach it to the function type. Ah, I see -- acquire can be on a function type, but use and release are on the parameter. You are missing some test cases: ``` int correct() [[clang::acquire_handle]]; // Should work, applies to the function type int (*fp [[clang::acquire_handle]])(); // Should work, applies to the function type ``` ================ Comment at: clang/test/Sema/attr-handles.cpp:3 + +// Decl annotations. +void f(int *a __attribute__((acquire_handle))); ---------------- Can you add some test cases showing that these work on a typedef declaration? 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