aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:2655
+
+def Retain : InheritableAttr {
+ let Spellings = [GCC<"retain">];
----------------
Should this be a target-specific attribute as it only has effects on ELF
targets?
================
Comment at: clang/lib/Sema/SemaDecl.cpp:2834
}
+ if (RetainAttr *OldAttr = Old->getMostRecentDecl()->getAttr<RetainAttr>()) {
+ RetainAttr *NewAttr = OldAttr->clone(Context);
----------------
I realize you're following the same pattern as the used attribute, but.. why is
this code even necessary? The attributes are already marked as being inherited
in Attr.td, so I'd not expect to need either of these. (If you don't know the
answer off the top of your head, that's fine -- I can investigate on my own.)
================
Comment at: clang/test/Sema/attr-retain.c:3
+
+extern char test1[] __attribute__((retain)); // expected-warning {{'retain'
attribute ignored}}
+extern const char test2[] __attribute__((retain)); // expected-warning
{{'retain' attribute ignored}}
----------------
Oof, this is not a particularly friendly diagnostic (there's no mention of
*why* the attribute is ignored, so it's hard to know how to respond to the
diagnostic as a user).
================
Comment at: clang/test/Sema/attr-retain.c:8
+void foo() {
+ int l __attribute__((retain)); // expected-warning {{'retain' attribute only
applies to variables with non-local storage, functions, and Objective-C
methods}}
+}
----------------
Can you also add a test case showing the attribute does not accept any
arguments?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96838/new/
https://reviews.llvm.org/D96838
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
