[PATCH] D49911: Summary:Add clang::reinitializes attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Should this attribute have some semantic checking that ensures the non-static data members are accessed in the function that claims it reinitializes the object? Otherwise, it seems like this would not trigger any diagnostics:

  class C {
    int a, b;
  public:
    [[clang::reinitializes]] void clear() {}
  };



================
Comment at: include/clang/Basic/Attr.td:96
+                    [{!S->isStatic() && !S->isConst()}],
+                    "non-static non-const member functions">;
+
----------------
`non-static, non-const member functions` (with the comma)


================
Comment at: include/clang/Basic/Attr.td:2945
+def Reinitializes : InheritableAttr {
+  let Spellings = [CXX11<"clang", "reinitializes">];
+  let Subjects = SubjectList<[NonStaticNonConstCXXMethod], ErrorDiag>;
----------------
I think it makes sense to use `CXX11` instead of `Clang` as this attribute doesn't make sense in C2x mode, but should there be a `GNU` spelling as well?


================
Comment at: include/clang/Basic/AttrDocs.td:3426
+  let Content = [{
+The ``reinitializes`` attribute can be applied to a non-static, non-const C++
+member function to indicate that this member function reinitializes the entire
----------------
While I kind of understand the restriction on a `const` member function, what about code that has mutable members being reset within a const method?


================
Comment at: test/SemaCXX/attr-reinitializes.cpp:3
+
+[[clang::reinitializes]] int a; // expected-error {{only applies to}}
+
----------------
Please spell out the full diagnostic the first time it appears, and shorten the other uses, if you'd like.


================
Comment at: test/SemaCXX/attr-reinitializes.cpp:9
+  [[clang::reinitializes]] void foo();
+  [[clang::reinitializes]] void bar() const; // expected-error {{only applies to non-static non-const}}
+  [[clang::reinitializes]] static void baz(); // expected-error {{only applies to}}
----------------
Please also add tests that demonstrate the attribute is intended to accept no args.


Repository:
  rC Clang

https://reviews.llvm.org/D49911