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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
