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

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:3445
+  let Spellings = [Clang<"acquire_handle">];
+  let Subjects = SubjectList<[Function, ParmVar], ErrorDiag>;
+  let Documentation = [AcquireHandleDocs];
----------------
aaron.ballman wrote:
> xazax.hun wrote:
> > aaron.ballman wrote:
> > > What about function-like interfaces such as lambdas, blocks, or other callable objects that are not a `FunctionDecl`?
> > Good point!
> This get you partway there -- `FunctionLike` allows functions and function pointers, but I don't think it allows lambdas or blocks. You should add some test cases for all of these (including function pointers) to verify you're able to mark the entities you want and get the behavior you're after.
> 
> For instance, on function pointers and lambdas, this may impact the function *type* which is a good design question to be thinking about. What should happen with code like:
> ```
> [[clang::acquire_handle]] void int open(const char *);
> int (*fp)(const char *) = open;
> ```
> If the attribute is a type attribute as opposed to a declaration attribute, then you can think about diagnostics in these sort of cases. However, if it is a type attribute, there is more work involved in hooking it up to the type system.
> 
> FWIW, this attribute smells like it should be a type attribute rather than a declaration attribute because I can imagine wanting to use this with function pointers and not losing the analysis capabilities (esp for things like callback functions).
I see your point. I have, however, some concerns making this part of the type system. I think it would be great to let the users add the annotations gradually without any diagnostic. For example:

```
void my_function_with_callback( int (*callback)(int __attribute__((handle_use)) );

...

my_function_with_callback(already_annotated);
my_function_with_callback(not_yet_annotated); // Do we intend to give a diagnostics here?
```

What do you think?


================
Comment at: clang/include/clang/Basic/Attr.td:3445
+  let Spellings = [Clang<"acquire_handle">];
+  let Subjects = SubjectList<[Function, ParmVar], ErrorDiag>;
+  let Documentation = [AcquireHandleDocs];
----------------
xazax.hun wrote:
> aaron.ballman wrote:
> > xazax.hun wrote:
> > > aaron.ballman wrote:
> > > > What about function-like interfaces such as lambdas, blocks, or other callable objects that are not a `FunctionDecl`?
> > > Good point!
> > This get you partway there -- `FunctionLike` allows functions and function pointers, but I don't think it allows lambdas or blocks. You should add some test cases for all of these (including function pointers) to verify you're able to mark the entities you want and get the behavior you're after.
> > 
> > For instance, on function pointers and lambdas, this may impact the function *type* which is a good design question to be thinking about. What should happen with code like:
> > ```
> > [[clang::acquire_handle]] void int open(const char *);
> > int (*fp)(const char *) = open;
> > ```
> > If the attribute is a type attribute as opposed to a declaration attribute, then you can think about diagnostics in these sort of cases. However, if it is a type attribute, there is more work involved in hooking it up to the type system.
> > 
> > FWIW, this attribute smells like it should be a type attribute rather than a declaration attribute because I can imagine wanting to use this with function pointers and not losing the analysis capabilities (esp for things like callback functions).
> I see your point. I have, however, some concerns making this part of the type system. I think it would be great to let the users add the annotations gradually without any diagnostic. For example:
> 
> ```
> void my_function_with_callback( int (*callback)(int __attribute__((handle_use)) );
> 
> ...
> 
> my_function_with_callback(already_annotated);
> my_function_with_callback(not_yet_annotated); // Do we intend to give a diagnostics here?
> ```
> 
> What do you think?
After some digging it looks like I cannot apply any attributes to lambdas. For example, you cannot even add `[[noreturn]]` to a lambda, because the language only supports type attributes in that position.


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

https://reviews.llvm.org/D70469