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

[Aaron Ballman via Phabricator via cfe-commits]

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