aaron.ballman added a comment. Some minor nits that can be handled post-commit.
================ Comment at: lib/Sema/SemaDeclAttr.cpp:5921 +static void handleDestroyAttr(Sema &S, Decl *D, const ParsedAttr &A) { + if (!isa<VarDecl>(D) || !cast<VarDecl>(D)->hasGlobalStorage()) { + S.Diag(D->getLocation(), diag::err_destroy_attr_on_non_static_var) ---------------- There is no need to check `VarDecl` here -- that's already done for you by the subject. ================ Comment at: lib/Sema/SemaDeclAttr.cpp:5927-5931 + if (A.getKind() == ParsedAttr::AT_AlwaysDestroy) { + handleSimpleAttributeWithExclusions<AlwaysDestroyAttr, NoDestroyAttr>(S, D, A); + } else { + handleSimpleAttributeWithExclusions<NoDestroyAttr, AlwaysDestroyAttr>(S, D, A); + } ---------------- Elide braces. ================ Comment at: test/SemaCXX/no_destroy.cpp:33-34 +int main() { + [[clang::no_destroy]] int p; // expected-error{{no_destroy attribute can only be applied to a variable with static or thread storage duration}} + [[clang::always_destroy]] int p2; // expected-error{{always_destroy attribute can only be applied to a variable with static or thread storage duration}} + [[clang::no_destroy]] static int p3; ---------------- Please also add tests demonstrating that the attribute does not appertain to things that don't look like variables (like a function or struct declaration) and that it does not accept arguments. Repository: rC Clang https://reviews.llvm.org/D50994 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits