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

[Aaron Ballman via Phabricator via cfe-commits]

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)
----------------
xazax.hun wrote:
> aaron.ballman wrote:
> > 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.
> I think we might consider it incorrect for the acquire as well. Because if we let the user put acquire on the function, it is ambiguous where to put the annotation. If we do not let them put on the function they are always forced to put on the return type to express this. But in case this kind of ambiguity is a feature and not a bug I can change the behavior.
I guess I was considering the semantics as being acquire on a function type applies to the non-void return value being the handle and release on a function type applies to the only parameter being passed to the function.

I don't think it makes sense to apply acquire or release semantics to a handle type itself because that's not really part of the type identity itself (it's not an "acquiring handle" or a "releasing handle" -- I would think it's *both* at the same time), so I think that's been throwing me for a loop with the latest round of patches here. What I was imagining was that you should mark any place a handle is created or released, which works almost-fine for parameters (because you can mark the parameter declarations with an attribute). It doesn't quite work for function pointers, however, because those don't have parameters. e.g., `void (*fp)([[clang::acquire_handle]] int &)` won't do what you expect, I believe. However, for functions which return the acquired handle, there is no good place to write the attribute except on the function itself (because, again, it's not a property of the return type, but of the specific function's return type). For those cases, I imagine you want the function *type* to be what codifies that a handle is returned, because then it will work with function pointers. For *using* a handle, I'm not certain what the benefit is to having an attribute on each use -- that seems likely for a user to forget to add an annotation somewhere and get incorrect behavior. I would normally suggest that the attribute be applied to the type declaration of the handle (e.g., through a typedef), but if you want this to work with handles coming from system headers (like file descriptors or sockets, etc) then will this approach work?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70469/new/

https://reviews.llvm.org/D70469