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 = [Clang<"acquire_handle">];
----------------
aaron.ballman wrote:
> 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
> }
> ```
I verified that this is an issue, but it's not a new one. 
`[[clang::lifetimebound]]` has the same issue. You should add a FIXME comment 
here and a test case with a FIXME comment so we don't lose track of this, but 
it doesn't seem critical to fix right now.


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