[cfe-commits] r162216 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h lib/StaticAnalyzer/Core/ExplodedGraph.cpp

[Jordan Rose]

Author: jrose
Date: Mon Aug 20 13:59:46 2012
New Revision: 162216

URL: http://llvm.org/viewvc/llvm-project?rev=162216&view=rev
Log:
[analyzer] Add comments to ExplodedNode::NodeGroup.

No functionality change.

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
    cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h?rev=162216&r1=162215&r2=162216&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h Mon Aug 20 13:59:46 2012
@@ -60,10 +60,20 @@
   friend class SwitchNodeBuilder;
   friend class EndOfFunctionNodeBuilder;
 
+  /// Efficiently stores a list of ExplodedNodes, or an optional flag.
+  ///
+  /// NodeGroup provides opaque storage for a list of ExplodedNodes, optimizing
+  /// for the case when there is only one node in the group. This is a fairly
+  /// common case in an ExplodedGraph, where most nodes have only one
+  /// predecessor and many have only one successor. It can also be used to
+  /// store a flag rather than a node list, which ExplodedNode uses to mark
+  /// whether a node is a sink. If the flag is set, the group is implicitly
+  /// empty and no nodes may be added.
   class NodeGroup {
     // Conceptually a discriminated union. If the low bit is set, the node is
     // a sink. If the low bit is not set, the pointer refers to the storage
     // for the nodes in the group.
+    // This is not a PointerIntPair in order to keep the storage type opaque.
     uintptr_t P;
     
   public:
@@ -79,10 +89,19 @@
 
     bool empty() const { return P == 0 || getFlag() != 0; }
 
+    /// Adds a node to the list.
+    ///
+    /// The group must not have been created with its flag set.
     void addNode(ExplodedNode *N, ExplodedGraph &G);
 
+    /// Replaces the single node in this group with a new node.
+    ///
+    /// Note that this should only be used when you know the group was not
+    /// created with its flag set, and that the group is empty or contains
+    /// only a single node.
     void replaceNode(ExplodedNode *node);
 
+    /// Returns whether this group was created with its flag set.
     bool getFlag() const {
       return (P & 1);
     }

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp?rev=162216&r1=162215&r2=162216&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp Mon Aug 20 13:59:46 2012
@@ -162,6 +162,16 @@
 // ExplodedNode.
 //===----------------------------------------------------------------------===//
 
+// An NodeGroup's storage type is actually very much like a TinyPtrVector:
+// it can be either a pointer to a single ExplodedNode, or a pointer to a
+// BumpVector allocated with the ExplodedGraph's allocator. This allows the
+// common case of single-node NodeGroups to be implemented with no extra memory.
+//
+// Consequently, each of the NodeGroup methods have up to four cases to handle:
+// 1. The flag is set and this group does not actually contain any nodes.
+// 2. The group is empty, in which case the storage value is null.
+// 3. The group contains a single node.
+// 4. The group contains more than one node.
 typedef BumpVector<ExplodedNode *> ExplodedNodeVector;
 typedef llvm::PointerUnion<ExplodedNode *, ExplodedNodeVector *> GroupStorage;
 
@@ -175,6 +185,8 @@
 }
 
 void ExplodedNode::NodeGroup::replaceNode(ExplodedNode *node) {
+  assert(!getFlag());
+
   GroupStorage &Storage = reinterpret_cast<GroupStorage&>(P);
   assert(Storage.is<ExplodedNode *>());
   Storage = node;