[analyzer] MisusedMovedObject: NFC: Remove dead code after D18860

[NoQ]

Summary

Since D18860 addresses the problem that some regions are not properly cleaned up, as explained in D18860#1294011, it's no longer necessary to clean them up manually. This patch removes the respective code from checkEndFunction and from checkPostCall for destructors. No functional change intended.

[NoQ]

Bring back the early return for destructors. Cleaning up the state might be unnecessary, but calling a destructor on a moved object is still fine and should not cause a warning.

Add a test because this wasn't caught by tests.

lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp

public:
   static RegionState getReported() { return RegionState(Reported); }
   static RegionState getMoved() { return RegionState(Moved); }
 
   bool operator==(const RegionState &X) const { return K == X.K; }
   void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(K); }
 };
 
 class MisusedMovedObjectChecker
-    : public Checker<check::PreCall, check::PostCall, check::EndFunction,
+    : public Checker<check::PreCall, check::PostCall,
                      check::DeadSymbols, check::RegionChanges> {
 public:
-  void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
   void checkPreCall(const CallEvent &MC, CheckerContext &C) const;
   void checkPostCall(const CallEvent &MC, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
   ProgramStateRef
   checkRegionChanges(ProgramStateRef State,
                      const InvalidatedSymbols *Invalidated,
                      ArrayRef<const MemRegion *> ExplicitRegions,
                      ArrayRef<const MemRegion *> Regions,
auto R =
                                      MoveNode->getLocationContext()->getDecl());
     R->addVisitor(llvm::make_unique<MovedBugVisitor>(Region));
     C.emitReport(std::move(R));
     return N;
   }
   return nullptr;
 }
 
-// Removing the function parameters' MemRegion from the state. This is needed
-// for PODs where the trivial destructor does not even created nor executed.
-void MisusedMovedObjectChecker::checkEndFunction(const ReturnStmt *RS,
-                                                 CheckerContext &C) const {
-  auto State = C.getState();
-  TrackedRegionMapTy Objects = State->get<TrackedRegionMap>();
-  if (Objects.isEmpty())
-    return;
-
-  auto LC = C.getLocationContext();
-
-  const auto LD = dyn_cast_or_null<FunctionDecl>(LC->getDecl());
-  if (!LD)
-    return;
-  llvm::SmallSet<const MemRegion *, 8> InvalidRegions;
-
-  for (auto Param : LD->parameters()) {
-    auto Type = Param->getType().getTypePtrOrNull();
-    if (!Type)
-      continue;
-    if (!Type->isPointerType() && !Type->isReferenceType()) {
-      InvalidRegions.insert(State->getLValue(Param, LC).getAsRegion());
-    }
-  }
-
-  if (InvalidRegions.empty())
-    return;
-
-  for (const auto &E : State->get<TrackedRegionMap>()) {
-    if (InvalidRegions.count(E.first->getBaseRegion()))
-      State = State->remove<TrackedRegionMap>(E.first);
-  }
-
-  C.addTransition(State);
-}
-
 void MisusedMovedObjectChecker::checkPostCall(const CallEvent &Call,
                                               CheckerContext &C) const {
   const auto *AFC = dyn_cast<AnyFunctionCall>(&Call);
   if (!AFC)
     return;
 
   ProgramStateRef State = C.getState();
   const auto MethodDecl = dyn_cast_or_null<CXXMethodDecl>(AFC->getDecl());
if (CtorDec && CtorDec->isCopyOrMoveConstructor()) {
                                                RegionState::getReported());
         }
       }
     }
     C.addTransition(State, N);
     return;
   }
 
+  // Calling a destructor on a moved object is fine.
+  if (isa<CXXDestructorCall>(&Call))
+    return;
+
   const auto IC = dyn_cast<CXXInstanceCall>(&Call);
   if (!IC)
     return;
   // In case of destructor call we do not track the object anymore.
   const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
   if (!ThisRegion)
     return;
 
-  if (dyn_cast_or_null<CXXDestructorDecl>(Call.getDecl())) {
-    State = removeFromState(State, ThisRegion);
-    C.addTransition(State);
-    return;
-  }
-
   const auto MethodDecl = dyn_cast_or_null<CXXMethodDecl>(IC->getDecl());
   if (!MethodDecl)
     return;
   // Checking assignment operators.
   bool OperatorEq = MethodDecl->isOverloadedOperator() &&
                     MethodDecl->getOverloadedOperator() == OO_Equal;
   // Remove the tracked object for every assignment operator, but report bug
   // only for move or copy assignment's argument.

test/Analysis/MisusedMovedObject.cpp

 }
 
 void checkLoopZombies() {
   while (true) {
     Empty e{};
     Empty f = std::move(e); // no-warning
   }
 }
+
+struct MoveOnlyWithDestructor {
+  MoveOnlyWithDestructor();
+  ~MoveOnlyWithDestructor();
+  MoveOnlyWithDestructor(const MoveOnlyWithDestructor &m) = delete;
+  MoveOnlyWithDestructor(MoveOnlyWithDestructor &&m);
+};
+
+MoveOnlyWithDestructor foo() {
+  MoveOnlyWithDestructor m;
+  return m;
+}