Cleanup

Author: steakhal

clang/include/clang/AST/AttrIterator.h

@@ -16,6 +16,7 @@
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/ADL.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Casting.h"
 #include <cassert>
 #include <cstddef>
@@ -124,6 +125,17 @@ inline auto *getSpecificAttr(const Container &container) {
   return It != specific_attr_end<IterTy>(container) ? *It : nullptr;
 }
 
+template <typename SpecificAttr, typename Container>
+inline auto getSpecificAttrs(const Container &container) {
+  using ValueTy = llvm::detail::ValueOfRange<Container>;
+  using ValuePointeeTy = std::remove_pointer_t<ValueTy>;
+  using IterTy = std::conditional_t<std::is_const_v<ValuePointeeTy>,
+                                    const SpecificAttr, SpecificAttr>;
+  auto Begin = specific_attr_begin<IterTy>(container);
+  auto End = specific_attr_end<IterTy>(container);
+  return llvm::make_range(Begin, End);
+}
+
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_ATTRITERATOR_H

clang/lib/Analysis/CFG.cpp

@@ -433,7 +433,7 @@ class reverse_children {
   ArrayRef<Stmt *> children;
 
 public:
-  reverse_children(Stmt *S, ASTContext *astContext = nullptr);
+  reverse_children(Stmt *S, ASTContext &Ctx);
 
   using iterator = ArrayRef<Stmt *>::reverse_iterator;
 
@@ -443,61 +443,47 @@ class reverse_children {
 
 } // namespace
 
-reverse_children::reverse_children(Stmt *S, ASTContext *AstC) {
-  if (CallExpr *CE = dyn_cast<CallExpr>(S)) {
-    children = CE->getRawSubExprs();
+reverse_children::reverse_children(Stmt *S, ASTContext &Ctx) {
+  switch (S->getStmtClass()) {
+  case Stmt::CallExprClass: {
+    children = cast<CallExpr>(S)->getRawSubExprs();
     return;
   }
-  switch (S->getStmtClass()) {
-    // Note: Fill in this switch with more cases we want to optimize.
-    case Stmt::InitListExprClass: {
-      InitListExpr *IE = cast<InitListExpr>(S);
-      children = llvm::ArrayRef(reinterpret_cast<Stmt **>(IE->getInits()),
-                                IE->getNumInits());
-      return;
-    }
-    case Stmt::AttributedStmtClass: {
-      assert(S->getStmtClass() == Stmt::AttributedStmtClass);
-      assert(AstC &&
-             "Attributes need the ast context to determine side-effects");
-      AttributedStmt *AS = cast<AttributedStmt>(S);
-      assert(AS);
-
-      // for an attributed stmt, the "children()" returns only the NullStmt
-      // (;) but semantically the "children" are supposed to be the
-      // expressions _within_ i.e. the two square brackets i.e. [[ HERE ]]
-      // so we add the subexpressions first, _then_ add the "children"
-      for (const Attr *Attr : AS->getAttrs()) {
-        // Only handles [[ assume(<assumption>) ]] right now
-        CXXAssumeAttr const *AssumeAttr = llvm::dyn_cast<CXXAssumeAttr>(Attr);
-        if (!AssumeAttr) {
-          continue;
-        }
-        Expr *AssumeExpr = AssumeAttr->getAssumption();
-        // If we skip adding the assumption expression to CFG,
-        // it doesn't get "branch"-ed by symbol analysis engine
-        // presumably because it's literally not in the CFG
 
-        if (AssumeExpr->HasSideEffects(*AstC)) {
-          continue;
+  // Note: Fill in this switch with more cases we want to optimize.
+  case Stmt::InitListExprClass: {
+    InitListExpr *IE = cast<InitListExpr>(S);
+    children = llvm::ArrayRef(reinterpret_cast<Stmt **>(IE->getInits()),
+                              IE->getNumInits());
+    return;
+  }
+  case Stmt::AttributedStmtClass: {
+    auto *AS = cast<AttributedStmt>(S);
+
+    // for an attributed stmt, the "children()" returns only the NullStmt
+    // (;) but semantically the "children" are supposed to be the
+    // expressions _within_ i.e. the two square brackets i.e. [[ HERE ]]
+    // so we add the subexpressions first, _then_ add the "children"
+
+    for (const auto *Attr : AS->getAttrs()) {
+      if (const auto *AssumeAttr = dyn_cast<CXXAssumeAttr>(Attr)) {
+        Expr *AssumeExpr = AssumeAttr->getAssumption();
+        if (!AssumeExpr->HasSideEffects(Ctx)) {
+          childrenBuf.push_back(AssumeExpr);
         }
-        childrenBuf.push_back(AssumeExpr);
       }
-      // children() for an CXXAssumeAttr is NullStmt(;)
-      // for others, it will have existing behavior
+      // Visit the actual children AST nodes.
+      // For CXXAssumeAttrs, this is always a NullStmt.
       llvm::append_range(childrenBuf, AS->children());
       children = childrenBuf;
-      return;
     }
-    default:
-      break;
+    return;
+  }
+  default:
+    // Default case for all other statements.
+    llvm::append_range(childrenBuf, S->children());
+    children = childrenBuf;
   }
-
-  // Default case for all other statements.
-  llvm::append_range(childrenBuf, S->children());
-
-  // This needs to be done *after* childrenBuf has been populated.
-  children = childrenBuf;
 }
 
 namespace {
@@ -2464,7 +2450,7 @@ CFGBlock *CFGBuilder::VisitChildren(Stmt *S) {
 
   // Visit the children in their reverse order so that they appear in
   // left-to-right (natural) order in the CFG.
-  reverse_children RChildren(S, Context);
+  reverse_children RChildren(S, *Context);
   for (Stmt *Child : RChildren) {
     if (Child)
       if (CFGBlock *R = Visit(Child))
@@ -2480,7 +2466,7 @@ CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
   }
   CFGBlock *B = Block;
 
-  reverse_children RChildren(ILE);
+  reverse_children RChildren(ILE, *Context);
   for (Stmt *Child : RChildren) {
     if (!Child)
       continue;

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

@@ -785,8 +785,7 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
                                   const Expr *R,
                                   ExplodedNode *Pred,
                                   ExplodedNodeSet &Dst) {
-  assert(L && "L does not exist!");
-  assert(R && "R does not exist!");
+  assert(L && R);
 
   StmtNodeBuilder B(Pred, Dst, *currBldrCtx);
   ProgramStateRef state = Pred->getState();
@@ -797,17 +796,16 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex,
   ProgramStateRef SrcState = state;
 
   for (const ExplodedNode *N = Pred ; N ; N = *N->pred_begin()) {
-    ProgramPoint PP = N->getLocation();
-    if (PP.getAs<PreStmtPurgeDeadSymbols>() || PP.getAs<BlockEntrance>() || PP.getAs<StmtPoint>()) {
+    auto Edge = N->getLocationAs<BlockEdge>();
+    if (!Edge.has_value()) {
       // If the state N has multiple predecessors P, it means that successors
       // of P are all equivalent.
       // In turn, that means that all nodes at P are equivalent in terms
       // of observable behavior at N, and we can follow any of them.
       // FIXME: a more robust solution which does not walk up the tree.
       continue;
     }
-    assert(PP.getAs<BlockEdge>());
-    SrcBlock = PP.castAs<BlockEdge>().getSrc();
+    SrcBlock = Edge->getSrc();
     SrcState = N->getState();
     break;
   }

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

@@ -10,23 +10,17 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/AST/ASTContext.h"
+#include "clang/AST/AttrIterator.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/StmtCXX.h"
-#include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/ConstructionContext.h"
-#include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Sequence.h"
 #include <optional>
@@ -1216,14 +1210,9 @@ void ExprEngine::VisitAttributedStmt(const AttributedStmt *A,
   ExplodedNodeSet EvalSet;
   StmtNodeBuilder Bldr(CheckerPreStmt, EvalSet, *currBldrCtx);
 
-  for (const auto *attr : A->getAttrs()) {
-    CXXAssumeAttr const *AssumeAttr = llvm::dyn_cast<CXXAssumeAttr>(attr);
-    if (!AssumeAttr) {
-      continue;
-    }
-    Expr *AssumeExpr = AssumeAttr->getAssumption();
+  for (const auto *Attr : getSpecificAttrs<CXXAssumeAttr>(A->getAttrs())) {
     for (ExplodedNode *N : CheckerPreStmt) {
-      Visit(AssumeExpr, N, EvalSet);
+      Visit(Attr->getAssumption(), N, EvalSet);
     }
   }
 

clang/test/Analysis/cxx23-assume-attribute.cpp

@@ -28,7 +28,7 @@ int ternary_in_builtin_assume(int a, int b) {
 int ternary_in_assume(int a, int b) {
   // FIXME notes
   // Currently, if this test is run without the core.builtin.Builtin checker, the above function with the __builtin_assume behaves identically to the following test
-  // i.e. calls to `clang_analyzer_dump` result in "extraneous"  prints of the SVal(s) `reg_$2<int > b ...` 
+  // i.e. calls to `clang_analyzer_dump` result in "extraneous"  prints of the SVal(s) `reg_$2<int > b ...`
   // as opposed to 4 or 10
   // which likely implies the Program State(s) did not get narrowed.
   // A new checker is likely needed to be implemented to properly handle the expressions within `[[assume]]` to eliminate the states where `b` is not narrowed.