aaron.ballman 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];
----------------
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).
================
Comment at: clang/include/clang/Basic/Attr.td:3451
+ let Spellings = [Clang<"use_handle">];
+ let Subjects = SubjectList<[ParmVar]>;
+ let Documentation = [UseHandleDocs];
----------------
One test that you should add is whether this case is properly handled:
```
void (*fp)(int whatever [[clang::uses_handle]]);
[](int whatever [[clang::uses_handle]]){};
```
e.g., when it's on a parameter declaration of function pointer type or a
lambda, does everything still behave as expected?
================
Comment at: clang/include/clang/Basic/AttrDocs.td:4473
+ let Content = [{
+Handles are a way to identify resources like files, sockets, processes. They
+are more opaque than pointers and widely used in system programming. They have
----------------
sockets, processes -> sockets, and processes
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6523
+ // Warn if the parameter is definitely not an output parameter.
+ if (auto *PVD = dyn_cast<ParmVarDecl>(D)) {
+ if (PVD->getType()->isIntegerType()) {
----------------
`const auto *`
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6524
+ if (auto *PVD = dyn_cast<ParmVarDecl>(D)) {
+ if (PVD->getType()->isIntegerType()) {
+ S.Diag(AL.getLoc(), diag::err_attribute_output_parameter)
----------------
I'm skeptical of this. I think a better check is if the type is a pointer or
reference -- was there a reason you didn't go with that approach?
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
