Skip to content

[StaticAnalyzer] Migrate away from PointerUnion::{is,get} (NFC) #118421

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

Conversation

kazutakahirata
Copy link
Contributor

Note that PointerUnion::{is,get} have been soft deprecated in
PointerUnion.h:

// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa, cast and the llvm::dyn_cast

I'm not touching PointerUnion::dyn_cast for now because it's a bit
complicated; we could blindly migrate it to dyn_cast_if_present, but
we should probably use dyn_cast when the operand is known to be
non-null.

Note that PointerUnion::{is,get} have been soft deprecated in
PointerUnion.h:

  // FIXME: Replace the uses of is(), get() and dyn_cast() with
  //        isa<T>, cast<T> and the llvm::dyn_cast<T>

I'm not touching PointerUnion::dyn_cast for now because it's a bit
complicated; we could blindly migrate it to dyn_cast_if_present, but
we should probably use dyn_cast when the operand is known to be
non-null.
@kazutakahirata kazutakahirata requested a review from nikic December 3, 2024 04:16
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Dec 3, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 3, 2024

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

Author: Kazu Hirata (kazutakahirata)

Changes

Note that PointerUnion::{is,get} have been soft deprecated in
PointerUnion.h:

// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa<T>, cast<T> and the llvm::dyn_cast<T>

I'm not touching PointerUnion::dyn_cast for now because it's a bit
complicated; we could blindly migrate it to dyn_cast_if_present, but
we should probably use dyn_cast when the operand is known to be
non-null.


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

5 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp (+2-2)
  • (modified) clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp (+4-4)
  • (modified) clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp (+5-5)
  • (modified) clang/lib/StaticAnalyzer/Core/MemRegion.cpp (+3-3)
  • (modified) clang/lib/StaticAnalyzer/Core/SVals.cpp (+7-7)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
index 9e81a6bd19fc5b..bfd31f25b26dc5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
@@ -211,8 +211,8 @@ class MallocSizeofChecker : public Checker<check::ASTCodeBody> {
           continue;
 
         const TypeSourceInfo *TSI = nullptr;
-        if (CallRec.CastedExprParent.is<const VarDecl *>()) {
-          TSI = CallRec.CastedExprParent.get<const VarDecl *>()
+        if (isa<const VarDecl *>(CallRec.CastedExprParent)) {
+          TSI = cast<const VarDecl *>(CallRec.CastedExprParent)
                     ->getTypeSourceInfo();
         } else {
           TSI = CallRec.ExplicitCastType;
diff --git a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
index b0563b6c070f1f..1f2e14424a264e 100644
--- a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -196,13 +196,13 @@ const PointerToMemberData *BasicValueFactory::accumCXXBase(
   const NamedDecl *ND = nullptr;
   llvm::ImmutableList<const CXXBaseSpecifier *> BaseSpecList;
 
-  if (PTMDT.isNull() || PTMDT.is<const NamedDecl *>()) {
-    if (PTMDT.is<const NamedDecl *>())
-      ND = PTMDT.get<const NamedDecl *>();
+  if (PTMDT.isNull() || isa<const NamedDecl *>(PTMDT)) {
+    if (isa<const NamedDecl *>(PTMDT))
+      ND = cast<const NamedDecl *>(PTMDT);
 
     BaseSpecList = CXXBaseListFactory.getEmptyList();
   } else {
-    const PointerToMemberData *PTMD = PTMDT.get<const PointerToMemberData *>();
+    const PointerToMemberData *PTMD = cast<const PointerToMemberData *>(PTMDT);
     ND = PTMD->getDeclaratorDecl();
 
     BaseSpecList = PTMD->getCXXBaseList();
diff --git a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
index 1e0cc2eea9ed85..5f9de64b7d4c36 100644
--- a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -211,9 +211,9 @@ void ExplodedNode::NodeGroup::replaceNode(ExplodedNode *node) {
   assert(!getFlag());
 
   GroupStorage &Storage = reinterpret_cast<GroupStorage&>(P);
-  assert(Storage.is<ExplodedNode *>());
+  assert(isa<ExplodedNode *>(Storage));
   Storage = node;
-  assert(Storage.is<ExplodedNode *>());
+  assert(isa<ExplodedNode *>(Storage));
 }
 
 void ExplodedNode::NodeGroup::addNode(ExplodedNode *N, ExplodedGraph &G) {
@@ -222,7 +222,7 @@ void ExplodedNode::NodeGroup::addNode(ExplodedNode *N, ExplodedGraph &G) {
   GroupStorage &Storage = reinterpret_cast<GroupStorage&>(P);
   if (Storage.isNull()) {
     Storage = N;
-    assert(Storage.is<ExplodedNode *>());
+    assert(isa<ExplodedNode *>(Storage));
     return;
   }
 
@@ -230,7 +230,7 @@ void ExplodedNode::NodeGroup::addNode(ExplodedNode *N, ExplodedGraph &G) {
 
   if (!V) {
     // Switch from single-node to multi-node representation.
-    ExplodedNode *Old = Storage.get<ExplodedNode *>();
+    ExplodedNode *Old = cast<ExplodedNode *>(Storage);
 
     BumpVectorContext &Ctx = G.getNodeAllocator();
     V = new (G.getAllocator()) ExplodedNodeVector(Ctx, 4);
@@ -238,7 +238,7 @@ void ExplodedNode::NodeGroup::addNode(ExplodedNode *N, ExplodedGraph &G) {
 
     Storage = V;
     assert(!getFlag());
-    assert(Storage.is<ExplodedNodeVector *>());
+    assert(isa<ExplodedNodeVector *>(Storage));
   }
 
   V->push_back(N, G.getNodeAllocator());
diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index ad4e43630dd44e..0c4785d4d4c07d 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1068,10 +1068,10 @@ const VarRegion *MemRegionManager::getVarRegion(const VarDecl *D,
     llvm::PointerUnion<const StackFrameContext *, const VarRegion *> V =
       getStackOrCaptureRegionForDeclContext(LC, DC, D);
 
-    if (V.is<const VarRegion*>())
-      return V.get<const VarRegion*>();
+    if (isa<const VarRegion *>(V))
+      return cast<const VarRegion *>(V);
 
-    const auto *STC = V.get<const StackFrameContext *>();
+    const auto *STC = cast<const StackFrameContext *>(V);
 
     if (!STC) {
       // FIXME: Assign a more sensible memory space to static locals
diff --git a/clang/lib/StaticAnalyzer/Core/SVals.cpp b/clang/lib/StaticAnalyzer/Core/SVals.cpp
index 84e7e033404c03..b738190940dfb0 100644
--- a/clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -205,10 +205,10 @@ const NamedDecl *nonloc::PointerToMember::getDecl() const {
     return nullptr;
 
   const NamedDecl *ND = nullptr;
-  if (PTMD.is<const NamedDecl *>())
-    ND = PTMD.get<const NamedDecl *>();
+  if (isa<const NamedDecl *>(PTMD))
+    ND = cast<const NamedDecl *>(PTMD);
   else
-    ND = PTMD.get<const PointerToMemberData *>()->getDeclaratorDecl();
+    ND = cast<const PointerToMemberData *>(PTMD)->getDeclaratorDecl();
 
   return ND;
 }
@@ -227,16 +227,16 @@ nonloc::CompoundVal::iterator nonloc::CompoundVal::end() const {
 
 nonloc::PointerToMember::iterator nonloc::PointerToMember::begin() const {
   const PTMDataType PTMD = getPTMData();
-  if (PTMD.is<const NamedDecl *>())
+  if (isa<const NamedDecl *>(PTMD))
     return {};
-  return PTMD.get<const PointerToMemberData *>()->begin();
+  return cast<const PointerToMemberData *>(PTMD)->begin();
 }
 
 nonloc::PointerToMember::iterator nonloc::PointerToMember::end() const {
   const PTMDataType PTMD = getPTMData();
-  if (PTMD.is<const NamedDecl *>())
+  if (isa<const NamedDecl *>(PTMD))
     return {};
-  return PTMD.get<const PointerToMemberData *>()->end();
+  return cast<const PointerToMemberData *>(PTMD)->end();
 }
 
 //===----------------------------------------------------------------------===//

@llvmbot
Copy link
Member

llvmbot commented Dec 3, 2024

@llvm/pr-subscribers-clang

Author: Kazu Hirata (kazutakahirata)

Changes

Note that PointerUnion::{is,get} have been soft deprecated in
PointerUnion.h:

// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa<T>, cast<T> and the llvm::dyn_cast<T>

I'm not touching PointerUnion::dyn_cast for now because it's a bit
complicated; we could blindly migrate it to dyn_cast_if_present, but
we should probably use dyn_cast when the operand is known to be
non-null.


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

5 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp (+2-2)
  • (modified) clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp (+4-4)
  • (modified) clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp (+5-5)
  • (modified) clang/lib/StaticAnalyzer/Core/MemRegion.cpp (+3-3)
  • (modified) clang/lib/StaticAnalyzer/Core/SVals.cpp (+7-7)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
index 9e81a6bd19fc5b..bfd31f25b26dc5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
@@ -211,8 +211,8 @@ class MallocSizeofChecker : public Checker<check::ASTCodeBody> {
           continue;
 
         const TypeSourceInfo *TSI = nullptr;
-        if (CallRec.CastedExprParent.is<const VarDecl *>()) {
-          TSI = CallRec.CastedExprParent.get<const VarDecl *>()
+        if (isa<const VarDecl *>(CallRec.CastedExprParent)) {
+          TSI = cast<const VarDecl *>(CallRec.CastedExprParent)
                     ->getTypeSourceInfo();
         } else {
           TSI = CallRec.ExplicitCastType;
diff --git a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
index b0563b6c070f1f..1f2e14424a264e 100644
--- a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -196,13 +196,13 @@ const PointerToMemberData *BasicValueFactory::accumCXXBase(
   const NamedDecl *ND = nullptr;
   llvm::ImmutableList<const CXXBaseSpecifier *> BaseSpecList;
 
-  if (PTMDT.isNull() || PTMDT.is<const NamedDecl *>()) {
-    if (PTMDT.is<const NamedDecl *>())
-      ND = PTMDT.get<const NamedDecl *>();
+  if (PTMDT.isNull() || isa<const NamedDecl *>(PTMDT)) {
+    if (isa<const NamedDecl *>(PTMDT))
+      ND = cast<const NamedDecl *>(PTMDT);
 
     BaseSpecList = CXXBaseListFactory.getEmptyList();
   } else {
-    const PointerToMemberData *PTMD = PTMDT.get<const PointerToMemberData *>();
+    const PointerToMemberData *PTMD = cast<const PointerToMemberData *>(PTMDT);
     ND = PTMD->getDeclaratorDecl();
 
     BaseSpecList = PTMD->getCXXBaseList();
diff --git a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
index 1e0cc2eea9ed85..5f9de64b7d4c36 100644
--- a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -211,9 +211,9 @@ void ExplodedNode::NodeGroup::replaceNode(ExplodedNode *node) {
   assert(!getFlag());
 
   GroupStorage &Storage = reinterpret_cast<GroupStorage&>(P);
-  assert(Storage.is<ExplodedNode *>());
+  assert(isa<ExplodedNode *>(Storage));
   Storage = node;
-  assert(Storage.is<ExplodedNode *>());
+  assert(isa<ExplodedNode *>(Storage));
 }
 
 void ExplodedNode::NodeGroup::addNode(ExplodedNode *N, ExplodedGraph &G) {
@@ -222,7 +222,7 @@ void ExplodedNode::NodeGroup::addNode(ExplodedNode *N, ExplodedGraph &G) {
   GroupStorage &Storage = reinterpret_cast<GroupStorage&>(P);
   if (Storage.isNull()) {
     Storage = N;
-    assert(Storage.is<ExplodedNode *>());
+    assert(isa<ExplodedNode *>(Storage));
     return;
   }
 
@@ -230,7 +230,7 @@ void ExplodedNode::NodeGroup::addNode(ExplodedNode *N, ExplodedGraph &G) {
 
   if (!V) {
     // Switch from single-node to multi-node representation.
-    ExplodedNode *Old = Storage.get<ExplodedNode *>();
+    ExplodedNode *Old = cast<ExplodedNode *>(Storage);
 
     BumpVectorContext &Ctx = G.getNodeAllocator();
     V = new (G.getAllocator()) ExplodedNodeVector(Ctx, 4);
@@ -238,7 +238,7 @@ void ExplodedNode::NodeGroup::addNode(ExplodedNode *N, ExplodedGraph &G) {
 
     Storage = V;
     assert(!getFlag());
-    assert(Storage.is<ExplodedNodeVector *>());
+    assert(isa<ExplodedNodeVector *>(Storage));
   }
 
   V->push_back(N, G.getNodeAllocator());
diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index ad4e43630dd44e..0c4785d4d4c07d 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1068,10 +1068,10 @@ const VarRegion *MemRegionManager::getVarRegion(const VarDecl *D,
     llvm::PointerUnion<const StackFrameContext *, const VarRegion *> V =
       getStackOrCaptureRegionForDeclContext(LC, DC, D);
 
-    if (V.is<const VarRegion*>())
-      return V.get<const VarRegion*>();
+    if (isa<const VarRegion *>(V))
+      return cast<const VarRegion *>(V);
 
-    const auto *STC = V.get<const StackFrameContext *>();
+    const auto *STC = cast<const StackFrameContext *>(V);
 
     if (!STC) {
       // FIXME: Assign a more sensible memory space to static locals
diff --git a/clang/lib/StaticAnalyzer/Core/SVals.cpp b/clang/lib/StaticAnalyzer/Core/SVals.cpp
index 84e7e033404c03..b738190940dfb0 100644
--- a/clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -205,10 +205,10 @@ const NamedDecl *nonloc::PointerToMember::getDecl() const {
     return nullptr;
 
   const NamedDecl *ND = nullptr;
-  if (PTMD.is<const NamedDecl *>())
-    ND = PTMD.get<const NamedDecl *>();
+  if (isa<const NamedDecl *>(PTMD))
+    ND = cast<const NamedDecl *>(PTMD);
   else
-    ND = PTMD.get<const PointerToMemberData *>()->getDeclaratorDecl();
+    ND = cast<const PointerToMemberData *>(PTMD)->getDeclaratorDecl();
 
   return ND;
 }
@@ -227,16 +227,16 @@ nonloc::CompoundVal::iterator nonloc::CompoundVal::end() const {
 
 nonloc::PointerToMember::iterator nonloc::PointerToMember::begin() const {
   const PTMDataType PTMD = getPTMData();
-  if (PTMD.is<const NamedDecl *>())
+  if (isa<const NamedDecl *>(PTMD))
     return {};
-  return PTMD.get<const PointerToMemberData *>()->begin();
+  return cast<const PointerToMemberData *>(PTMD)->begin();
 }
 
 nonloc::PointerToMember::iterator nonloc::PointerToMember::end() const {
   const PTMDataType PTMD = getPTMData();
-  if (PTMD.is<const NamedDecl *>())
+  if (isa<const NamedDecl *>(PTMD))
     return {};
-  return PTMD.get<const PointerToMemberData *>()->end();
+  return cast<const PointerToMemberData *>(PTMD)->end();
 }
 
 //===----------------------------------------------------------------------===//

Copy link
Contributor

@steakhal steakhal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Kazu
I always welcome your patches! True gems.

I left a couple of comments, mostly about following llvm style guides.

Copy link

github-actions bot commented Dec 3, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@kazutakahirata
Copy link
Contributor Author

@steakhal Thank you for the review! Please take a look at the revised patch.

Copy link
Contributor

@steakhal steakhal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome!
Thanks

@kazutakahirata kazutakahirata merged commit a9bf16d into llvm:main Dec 3, 2024
8 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_001_PointerUnion_StaticAnalyzer branch December 3, 2024 21:53
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.

3 participants