aaron.ballman added inline comments. ================ Comment at: include/clang/Basic/Attr.td:1384 @@ +1383,3 @@ +def RequireConstantInit : InheritableAttr { + let Spellings = [GCC<"require_constant_initialization">]; + let Subjects = SubjectList<[Var]>; ---------------- This should not use the GCC spelling because GCC doesn't support the attribute. Do you expect this attribute to do anything useful in C? If not, this should just be a CXX11 spelling in the clang namespace.
================ Comment at: include/clang/Basic/Attr.td:1385 @@ +1384,3 @@ + let Spellings = [GCC<"require_constant_initialization">]; + let Subjects = SubjectList<[Var]>; + let Documentation = [RequireConstantInitDocs]; ---------------- You might want to make a custom `SubsetSubject` for this. Check out `NormalVar`, `SharedVar`, and the likes in Attr.td. Then you can get rid of the custom handling in SemaDeclAttr.cpp. ================ Comment at: include/clang/Basic/AttrDocs.td:835 @@ +834,3 @@ + let Category = DocCatVariable; + let Heading = "require_constant_initialization"; + let Content = [{ ---------------- Should be no need to specify the heading manually since there's only one spelling for the attribute. ================ Comment at: include/clang/Basic/AttrDocs.td:847 @@ +846,3 @@ + +This attribute acts an a compile time assertion that the requirements +for constant initialization have been met. Since these requirements change ---------------- s/acts an a/acts as a ================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6842 @@ +6841,3 @@ +def err_require_constant_init_var_not_static_tls : Error< + "'require_constant_initialization' attribute can only be applied to variables " + "static or thread-local storage duration">; ---------------- I would reword this to: "'require_constant_initialization' attribute only applies to variables with static or thread-local storage duration" ================ Comment at: test/SemaCXX/attr-require-constant-initialization.cpp:1 @@ +1,2 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -fcxx-exceptions -std=c++03 %s +// RUN: %clang_cc1 -fsyntax-only -verify -fcxx-exceptions -std=c++11 %s ---------------- This file should also get tests that ensure we properly diagnose the attribute when it doesn't apply to a declaration (such as a local variable, as well as nonsense like a function), test that it does not accept arguments, etc. ================ Comment at: test/SemaCXX/attr-require-constant-initialization.cpp:4-6 @@ +3,5 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -fcxx-exceptions -std=c++14 %s +// +// Tests for "expression traits" intrinsics such as __is_lvalue_expr. +// + ---------------- Does this comment actually apply? https://reviews.llvm.org/D23385 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits