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

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348208: [analyzer] MoveChecker: NFC: Remove the workaround 
for the zombie symbols bug. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54372?vs=173779=176489#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54372/new/

https://reviews.llvm.org/D54372

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  cfe/trunk/test/Analysis/use-after-move.cpp

Index: cfe/trunk/test/Analysis/use-after-move.cpp
===
--- cfe/trunk/test/Analysis/use-after-move.cpp
+++ cfe/trunk/test/Analysis/use-after-move.cpp
@@ -714,3 +714,15 @@
 Empty f = std::move(e); // no-warning
   }
 }
+
+struct MoveOnlyWithDestructor {
+  MoveOnlyWithDestructor();
+  ~MoveOnlyWithDestructor();
+  MoveOnlyWithDestructor(const MoveOnlyWithDestructor ) = delete;
+  MoveOnlyWithDestructor(MoveOnlyWithDestructor &);
+};
+
+MoveOnlyWithDestructor foo() {
+  MoveOnlyWithDestructor m;
+  return m;
+}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -43,7 +43,7 @@
 };
 
 class MoveChecker
-: public Checker {
 public:
   void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
@@ -217,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 MoveChecker::checkEndFunction(const ReturnStmt *RS,
-   CheckerContext ) const {
-  auto State = C.getState();
-  TrackedRegionMapTy Objects = State->get();
-  if (Objects.isEmpty())
-return;
-
-  auto LC = C.getLocationContext();
-
-  const auto LD = dyn_cast_or_null(LC->getDecl());
-  if (!LD)
-return;
-  llvm::SmallSet 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  : State->get()) {
-if (InvalidRegions.count(E.first->getBaseRegion()))
-  State = State->remove(E.first);
-  }
-
-  C.addTransition(State);
-}
-
 void MoveChecker::checkPostCall(const CallEvent ,
 CheckerContext ) const {
   const auto *AFC = dyn_cast();
@@ -382,20 +346,19 @@
   const auto IC = dyn_cast();
   if (!IC)
 return;
-  // In case of destructor call we do not track the object anymore.
-  const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
-  if (!ThisRegion)
+
+  // Calling a destructor on a moved object is fine.
+  if (isa(IC))
 return;
 
-  if (dyn_cast_or_null(Call.getDecl())) {
-State = removeFromState(State, ThisRegion);
-C.addTransition(State);
+  const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
+  if (!ThisRegion)
 return;
-  }
 
   const auto MethodDecl = dyn_cast_or_null(IC->getDecl());
   if (!MethodDecl)
 return;
+
   // Checking assignment operators.
   bool OperatorEq = MethodDecl->isOverloadedOperator() &&
 MethodDecl->getOverloadedOperator() == OO_Equal;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

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 ) = delete;
+  MoveOnlyWithDestructor(MoveOnlyWithDestructor &);
+};
+
+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 {
 public:
-  void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
   void checkPreCall(const CallEvent , CheckerContext ) const;
   void checkPostCall(const CallEvent , CheckerContext ) const;
   void checkDeadSymbols(SymbolReaper , CheckerContext ) 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 ) const {
-  auto State = C.getState();
-  TrackedRegionMapTy Objects = State->get();
-  if (Objects.isEmpty())
-return;
-
-  auto LC = C.getLocationContext();
-
-  const auto LD = dyn_cast_or_null(LC->getDecl());
-  if (!LD)
-return;
-  llvm::SmallSet 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  : State->get()) {
-if (InvalidRegions.count(E.first->getBaseRegion()))
-  State = State->remove(E.first);
-  }
-
-  C.addTransition(State);
-}
-
 void MisusedMovedObjectChecker::checkPostCall(const CallEvent ,
   CheckerContext ) const {
   const auto *AFC = dyn_cast();
@@ -384,20 +347,18 @@
 return;
   }
 
+  // Calling a destructor on a moved object is fine.
+  if (isa())
+return;
+
   const auto IC = dyn_cast();
   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(Call.getDecl())) {
-State = removeFromState(State, ThisRegion);
-C.addTransition(State);
-return;
-  }
-
   const auto MethodDecl = dyn_cast_or_null(IC->getDecl());
   if (!MethodDecl)
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, baloghadamsoftware.

Since https://reviews.llvm.org/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.


Repository:
  rC Clang

https://reviews.llvm.org/D54372

Files:
  lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp


Index: lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
@@ -43,10 +43,9 @@
 };
 
 class MisusedMovedObjectChecker
-: public Checker {
 public:
-  void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
   void checkPreCall(const CallEvent , CheckerContext ) const;
   void checkPostCall(const CallEvent , CheckerContext ) const;
   void checkDeadSymbols(SymbolReaper , CheckerContext ) 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 ) const {
-  auto State = C.getState();
-  TrackedRegionMapTy Objects = State->get();
-  if (Objects.isEmpty())
-return;
-
-  auto LC = C.getLocationContext();
-
-  const auto LD = dyn_cast_or_null(LC->getDecl());
-  if (!LD)
-return;
-  llvm::SmallSet 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  : State->get()) {
-if (InvalidRegions.count(E.first->getBaseRegion()))
-  State = State->remove(E.first);
-  }
-
-  C.addTransition(State);
-}
-
 void MisusedMovedObjectChecker::checkPostCall(const CallEvent ,
   CheckerContext ) const {
   const auto *AFC = dyn_cast();
@@ -392,12 +355,6 @@
   if (!ThisRegion)
 return;
 
-  if (dyn_cast_or_null(Call.getDecl())) {
-State = removeFromState(State, ThisRegion);
-C.addTransition(State);
-return;
-  }
-
   const auto MethodDecl = dyn_cast_or_null(IC->getDecl());
   if (!MethodDecl)
 return;


Index: lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
@@ -43,10 +43,9 @@
 };
 
 class MisusedMovedObjectChecker
-: public Checker {
 public:
-  void checkEndFunction(const ReturnStmt *RS, CheckerContext ) const;
   void checkPreCall(const CallEvent , CheckerContext ) const;
   void checkPostCall(const CallEvent , CheckerContext ) const;
   void checkDeadSymbols(SymbolReaper , CheckerContext ) 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 ) const {
-  auto State = C.getState();
-  TrackedRegionMapTy Objects = State->get();
-  if (Objects.isEmpty())
-return;
-
-  auto LC = C.getLocationContext();
-
-  const auto LD = dyn_cast_or_null(LC->getDecl());
-  if (!LD)
-return;
-  llvm::SmallSet 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  : State->get()) {
-if (InvalidRegions.count(E.first->getBaseRegion()))
-  State = State->remove(E.first);
-  }
-
-  C.addTransition(State);
-}
-
 void MisusedMovedObjectChecker::checkPostCall(const CallEvent ,
   CheckerContext ) const {
   const auto *AFC = dyn_cast();
@@ -392,12 +355,6 @@
   if (!ThisRegion)
 return;
 
-  if (dyn_cast_or_null(Call.getDecl())) {
-State = removeFromState(State, ThisRegion);
-C.addTransition(State);
-