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 either) -- how bad is it that this is `DeclOrTypeAttr` in the case where 
it's an inherited parameter with the attribute? The other attributes use 
`InheritableParamAttr`, but the only thing they apply to is a parameter so 
that's easy. I don't know what should be done in this case, or if it's an issue.

I think a test case for this scenario would be:
```
void func([[clang::acquires_handle("foo") int *a);

void func(int *a) {
  // a should be marked as acquires_handle in the AST should you do an AST dump
}
```


================
Comment at: clang/lib/Sema/SemaType.cpp:7618
+    case ParsedAttr::AT_AcquireHandle: {
+      if (!type->isFunctionType())
+        return;
----------------
Can you be sure to add a test where the attribute is written in the type 
position for a non-function type to ensure you get a reasonable diagnostic?

e.g.,  `int [[clang::acquire_handle("")]] a;`


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

Reply via email to