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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits