[analyzer] Support Reinitializes attribute in MisusedMovedObject check

[xazax.hun]

Summary

Support the same attribute that the tidy version of this check already supports.
Also do some minor cleanups.

[NoQ]

Yay, these look useful. Is there also an attribute for methods that should never be called on a 'moved-from' object?

[xazax.hun]

In D52983#1258466, @NoQ wrote:

Yay, these look useful. Is there also an attribute for methods that should never be called on a 'moved-from' object?

I do not know about such attribute, but once contracts are implemented and wide-spread, a precondition on a method/function is a strong suggestion that it should not be used on a moved-from object.

[NoQ]

In D52983#1258499, @xazax.hun wrote:

In D52983#1258466, @NoQ wrote:

Yay, these look useful. Is there also an attribute for methods that should never be called on a 'moved-from' object?

I do not know about such attribute, but once contracts are implemented and wide-spread, a precondition on a method/function is a strong suggestion that it should not be used on a moved-from object.

Well, that actually sounds like a pretty good heuristic, as long as we know that a moved-from object cannot reliably satisfy these contracts. In other words, it'll probably be fine for most library classes, but my concern is that the more contracts do we document this way, the more false positives would we have, which is not a healthy correlation. Like, for example, if we add a contract "the object is in a consistent state" to all methods of an STL object, eg. something like this->_length == strlen(this->_buf) for std::string methods, it might be a valid contract, but all moved-from objects would trivially satisfy it (because they are guaranteed to be in a consistent albeit unspecified state), so we won't be able to all any method at all.

cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp

if (const auto *ConversionDec =
           dyn_cast_or_null<CXXConversionDecl>(MethodDec)) {
     const Type *Tp = ConversionDec->getConversionType().getTypePtrOrNull();
     if (!Tp)
       return false;
     if (Tp->isBooleanType() || Tp->isVoidType() || Tp->isVoidPointerType())
       return true;
   }
   // Function call `empty` can be skipped.
-  if (MethodDec && MethodDec->getDeclName().isIdentifier() &&
+  return (MethodDec && MethodDec->getDeclName().isIdentifier() &&
       (MethodDec->getName().lower() == "empty" ||
-       MethodDec->getName().lower() == "isempty"))
-    return true;
-
-  return false;
+       MethodDec->getName().lower() == "isempty"));
 }
 
 bool MisusedMovedObjectChecker::isStateResetMethod(
     const CXXMethodDecl *MethodDec) const {
-  if (MethodDec && MethodDec->getDeclName().isIdentifier()) {
+  if (!MethodDec)
+      return false;
+  if (MethodDec->hasAttr<ReinitializesAttr>())
+      return true;
+  if (MethodDec->getDeclName().isIdentifier()) {
     std::string MethodName = MethodDec->getName().lower();
     if (MethodName == "reset" || MethodName == "clear" ||
         MethodName == "destroy")
       return true;
   }
   return false;
 }
 
if (OperatorEq) {
     C.addTransition(State, N);
     return;
   }
 
   // The remaining part is check only for method call on a moved-from object.
 
   // We want to investigate the whole object, not only sub-object of a parent
   // class in which the encountered method defined.
-  while (const CXXBaseObjectRegion *BR =
-             dyn_cast<CXXBaseObjectRegion>(ThisRegion))
+  while (const auto *BR = dyn_cast<CXXBaseObjectRegion>(ThisRegion))
     ThisRegion = BR->getSuperRegion();
 
   if (isMoveSafeMethod(MethodDecl))
     return;
 
   if (isStateResetMethod(MethodDecl)) {
     State = removeFromState(State, ThisRegion);
     C.addTransition(State);
ProgramStateRef MisusedMovedObjectChecker::checkRegionChanges(
     const CallEvent *Call) const {
   // In case of an InstanceCall don't remove the ThisRegion from the GDM since
   // it is handled in checkPreCall and checkPostCall.
   const MemRegion *ThisRegion = nullptr;
   if (const auto *IC = dyn_cast_or_null<CXXInstanceCall>(Call)) {
     ThisRegion = IC->getCXXThisVal().getAsRegion();
   }
 
-  for (ArrayRef<const MemRegion *>::iterator I = ExplicitRegions.begin(),
-                                             E = ExplicitRegions.end();
-       I != E; ++I) {
-    const auto *Region = *I;
-    if (ThisRegion != Region) {
+  for (const auto *Region : ExplicitRegions) {
+    if (ThisRegion != Region)
       State = removeFromState(State, Region);
-    }
-  }
 
   return State;
 }
 
 void MisusedMovedObjectChecker::printState(raw_ostream &Out,
                                            ProgramStateRef State,
                                            const char *NL,
                                            const char *Sep) const {

cfe/trunk/test/Analysis/MisusedMovedObject.cpp

void ifStmtSequencesDeclAndConditionTest() {
   for (int i = 0; i < 3; ++i) {
     if (A a = A()) {
       A b;
       b = std::move(a); // no-warning
     }
   }
 }
 
-class C : public A {};
+struct C : public A {
+  [[clang::reinitializes]] void reinit();
+};
+
 void subRegionMoveTest() {
   {
     A a;
     B b = std::move(a.b); // expected-note {{'b' became 'moved-from' here}}
     a.b.foo();            // expected-warning {{Method call on a 'moved-from' object 'b'}} expected-note {{Method call on a 'moved-from' object 'b'}}
   }
   {
     A a;

 
 void resetSuperClass() {
   C c;
   C c1 = std::move(c);
   c.clear();
   C c2 = c; // no-warning
 }
 
+void resetSuperClass2() {
+  C c;
+  C c1 = std::move(c);
+  c.reinit();
+  C c2 = c; // no-warning
+}
+
 void reportSuperClass() {
   C c;
   C c1 = std::move(c); // expected-note {{'c' became 'moved-from' here}}
   c.foo();             // expected-warning {{Method call on a 'moved-from' object 'c'}} expected-note {{Method call on a 'moved-from' object 'c'}}
   C c2 = c;            // no-warning
 }