NoQ updated this revision to Diff 173779. NoQ added a comment. 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. https://reviews.llvm.org/D54372 Files: lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp test/Analysis/MisusedMovedObject.cpp
Index: test/Analysis/MisusedMovedObject.cpp =================================================================== --- test/Analysis/MisusedMovedObject.cpp +++ test/Analysis/MisusedMovedObject.cpp @@ -710,3 +710,15 @@ 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; +} Index: lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp @@ -43,10 +43,9 @@ }; 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; @@ -218,42 +217,6 @@ 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); @@ -384,20 +347,18 @@ 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;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits