fcloutier added inline comments.
================
Comment at: clang/test/AST/ast-print-attr.c:31
+// CHECK: int fun_annotate() __attribute__((annotate("annotation")))
+int fun_annotate() __attribute__((annotate("annotation")));
----------------
aaron.ballman wrote:
> Can you add a second test that shows we properly print the comma? e.g., `int
> fun_annotate2() __attribute__((annotate("annotation one", "annotation
> two")));`
I switched this to use `ownership_holds` and `ownership_returns` because
`annotate` has a `VariadicExprArgument` and it doesn't know how to print
expressions (it just prints a pointer value). `ownership_holds` and
`ownership_returns` are the same attribute, and they have the same
bug-triggering configuration (a fixed argument followed by a possibly-empty
list):
```c++
int *fun_returns() __attribute__((ownership_returns(fun_returns)));
void fun_holds(int *a) __attribute__((ownership_holds(fun_holds, 1)));
```
In the `ownership_returns` case, without my change, Clang prints
`__attribute__((ownership_returns(foo, )));`.
================
Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:2252
+ // Helper to print the starting character of an attribute argument. If there
+ // hasn't been an argument yet, it prints an opening parenthese; otherwise it
+ // prints a comma.
----------------
aaron.ballman wrote:
> One downside to printing the opening paren is that this can't be used in a
> generic way for generating any comma-separate list. That said, I think this
> functionality is clean -- perhaps renaming the function from `Comma` to
> `PrintAttributeArgListElement` or something would be an improvement?
I went with `DelimitAttributeArgument`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95695/new/
https://reviews.llvm.org/D95695
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
