Skip to content

[analyzer][NFC] Eliminate a dyn_cast #100719

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 5, 2024
Merged

Conversation

Szelethus
Copy link
Contributor

@llvmbot llvmbot added the clang Clang issues not falling into any other category label Jul 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 26, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Kristóf Umann (Szelethus)

Changes

Response to the catch in this comment: https://github.com/llvm/llvm-project/pull/94357/files/07f6daf2cf0f5d5bd4fc9950f2585a3f52b4ad2f#r1692084074


Full diff: https://github.com/llvm/llvm-project/pull/100719.diff

3 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (+2-3)
  • (modified) clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h (+1-1)
  • (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+2-3)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index fe202c79ed620..39e5c7c014a2a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -796,14 +796,13 @@ class NoMemOwnershipChangeVisitor final : public NoOwnershipChangeVisitor {
   /// done syntactically, because we are trying to argue about alternative
   /// paths of execution, and as a consequence we don't have path-sensitive
   /// information.
-  bool doesFnIntendToHandleOwnership(const Decl *Callee,
+  bool doesFnIntendToHandleOwnership(const FunctionDecl *Callee,
                                      ASTContext &ACtx) final {
     using namespace clang::ast_matchers;
-    const FunctionDecl *FD = dyn_cast<FunctionDecl>(Callee);
 
     auto Matches = match(findAll(stmt(anyOf(cxxDeleteExpr().bind("delete"),
                                             callExpr().bind("call")))),
-                         *FD->getBody(), ACtx);
+                         Callee->getBody(), ACtx);
     for (BoundNodes Match : Matches) {
       if (Match.getNodeAs<CXXDeleteExpr>("delete"))
         return true;
diff --git a/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h b/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
index 027f1a156a7c0..7be74860d863b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
+++ b/clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h
@@ -26,7 +26,7 @@ class NoOwnershipChangeVisitor : public NoStateChangeFuncVisitor {
   /// is done syntactically, because we are trying to argue about alternative
   /// paths of execution, and as a consequence we don't have path-sensitive
   /// information.
-  virtual bool doesFnIntendToHandleOwnership(const Decl *Callee,
+  virtual bool doesFnIntendToHandleOwnership(const FunctionDecl *Callee,
                                              ASTContext &ACtx) = 0;
 
   virtual bool hasResourceStateChanged(ProgramStateRef CallEnterState,
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 9aee7f952ad2d..41187ee9b5cdf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -746,13 +746,12 @@ class NoStreamStateChangeVisitor final : public NoOwnershipChangeVisitor {
     return StreamChk->FCloseDesc.matchesAsWritten(Call);
   }
 
-  bool doesFnIntendToHandleOwnership(const Decl *Callee,
+  bool doesFnIntendToHandleOwnership(const FunctionDecl *Callee,
                                      ASTContext &ACtx) final {
     using namespace clang::ast_matchers;
-    const FunctionDecl *FD = dyn_cast<FunctionDecl>(Callee);
 
     auto Matches =
-        match(findAll(callExpr().bind("call")), *FD->getBody(), ACtx);
+        match(findAll(callExpr().bind("call")), Callee->getBody(), ACtx);
     for (BoundNodes Match : Matches) {
       if (const auto *Call = Match.getNodeAs<CallExpr>("call"))
         if (isClosingCallAsWritten(*Call))

@steakhal
Copy link
Contributor

Can callables other than FunctionDecls take ownership? I'm thinking of ObjCMethodDecl or BlockDecl in particular - because they are not FunctionDecls.

@Szelethus Szelethus merged commit 8370ba4 into llvm:main Aug 5, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants