r180746 - Revert "[analyzer] Change PathPieces to be a wrapper around an ilist of (through indirection) PathDiagnosticPieces."

Ted Kremenek kremenek at apple.com
Mon Apr 29 16:12:59 PDT 2013


Author: kremenek
Date: Mon Apr 29 18:12:59 2013
New Revision: 180746

URL: http://llvm.org/viewvc/llvm-project?rev=180746&view=rev
Log:
Revert "[analyzer] Change PathPieces to be a wrapper around an ilist of (through indirection) PathDiagnosticPieces."

Jordan rightly pointed out that we can do the same with std::list.

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=180746&r1=180745&r2=180746&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h Mon Apr 29 18:12:59 2013
@@ -20,8 +20,6 @@
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerUnion.h"
-#include "llvm/ADT/ilist.h"
-#include "llvm/ADT/ilist_node.h"
 #include <deque>
 #include <iterator>
 #include <string>
@@ -388,32 +386,12 @@ public:
 
   virtual void Profile(llvm::FoldingSetNodeID &ID) const;
 };
-
-/// \brief An ordered collection of PathDiagnosticPieces.
-///
-/// Multiple PathPieces are allowed to reference the same PathDiagnosticPieces.
-/// This sharing is needed for some clients that want "flattened" copies
-/// of the same pieces.
-class PathPieces {
-  /// A simple wrapper for PathDiagnosticPiece, allowing sharing of
-  /// the same pieces between different PathPieces.
-  struct Node : public llvm::ilist_node<Node> {
-    IntrusiveRefCntPtr<PathDiagnosticPiece> Data;
-    explicit Node(PathDiagnosticPiece *P) : Data(P) {}
-    explicit Node() {}
-  };
-  llvm::ilist<Node> L;
-
+  
+  
+class PathPieces : public std::deque<IntrusiveRefCntPtr<PathDiagnosticPiece> > {
   void flattenTo(PathPieces &Primary, PathPieces &Current,
                  bool ShouldFlattenMacros) const;
-
 public:
-  PathPieces() {}
-  PathPieces &operator=(const PathPieces & X);
-  PathPieces(const PathPieces &X) {
-    *this = X;
-  }
-
   ~PathPieces();
 
   PathPieces flatten(bool ShouldFlattenMacros) const {
@@ -421,63 +399,6 @@ public:
     flattenTo(Result, Result, ShouldFlattenMacros);
     return Result;
   }
-
-  class iterator {
-    typedef llvm::ilist<Node>::iterator impl_iterator;
-    friend class PathPieces;
-    impl_iterator Impl;
-    iterator(const impl_iterator &Impl) : Impl(Impl) {}
-  public:
-    typedef PathDiagnosticPiece value_type;
-    typedef value_type* pointer;
-    typedef value_type& reference;
-    typedef ptrdiff_t difference_type;
-    typedef std::bidirectional_iterator_tag iterator_category;
-
-    bool operator==(const iterator &X) const {
-      return Impl == X.Impl;
-    }
-
-    bool operator!=(const iterator &X) const {
-      return Impl != X.Impl;
-    }
-
-    reference operator*() const { return *Impl->Data; }
-    pointer operator->() const { return Impl->Data.getPtr(); }
-
-    iterator &operator++() {
-      ++Impl;
-      return *this;
-    }
-
-    iterator &operator--() {
-      --Impl;
-      return *this;
-    }
-  };
-
-  typedef std::reverse_iterator<iterator> reverse_iterator;
-
-  iterator begin() const { return iterator(const_cast<PathPieces*>(this)->L.begin()); }
-  iterator end() const { return iterator(const_cast<PathPieces*>(this)->L.end()); }
-  reverse_iterator rbegin() { return reverse_iterator(end()); }
-  reverse_iterator rend() { return reverse_iterator(begin()); }
-
-  void push_front(PathDiagnosticPiece *P) {
-    L.push_front(new Node(P));
-  }
-  void pop_front() { L.pop_front(); }
-
-  void push_back(PathDiagnosticPiece *P) { L.push_back(new Node(P)); }
-  void push_back(const IntrusiveRefCntPtr<PathDiagnosticPiece> &P) {
-    push_back(P.getPtr());
-  }
-
-  PathDiagnosticPiece *front() const { return L.front().Data.getPtr(); }
-  PathDiagnosticPiece *back() const { return L.back().Data.getPtr(); }
-  void clear() { L.clear(); }
-  bool empty() const { return L.empty(); }
-  unsigned size() const { return L.size(); }
 };
 
 class PathDiagnosticSpotPiece : public PathDiagnosticPiece {
@@ -647,7 +568,7 @@ public:
     callEnter.flatten();
     callReturn.flatten();
     for (PathPieces::iterator I = path.begin(), 
-         E = path.end(); I != E; ++I) I->flattenLocations();
+         E = path.end(); I != E; ++I) (*I)->flattenLocations();
   }
   
   static PathDiagnosticCallPiece *construct(const ExplodedNode *N,
@@ -734,7 +655,7 @@ public:
   virtual void flattenLocations() {
     PathDiagnosticSpotPiece::flattenLocations();
     for (PathPieces::iterator I = subPieces.begin(), 
-         E = subPieces.end(); I != E; ++I) I->flattenLocations();
+         E = subPieces.end(); I != E; ++I) (*I)->flattenLocations();
   }
 
   static inline bool classof(const PathDiagnosticPiece *P) {
@@ -772,7 +693,7 @@ public:
 
   ~PathDiagnostic();
   
-  PathPieces &path;
+  const PathPieces &path;
 
   /// Return the path currently used by builders for constructing the 
   /// PathDiagnostic.
@@ -843,7 +764,7 @@ public:
   void flattenLocations() {
     Loc.flatten();
     for (PathPieces::iterator I = pathImpl.begin(), E = pathImpl.end(); 
-         I != E; ++I) I->flattenLocations();
+         I != E; ++I) (*I)->flattenLocations();
   }
 
   /// Profiles the diagnostic, independent of the path it references.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=180746&r1=180745&r2=180746&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Mon Apr 29 18:12:59 2013
@@ -123,7 +123,7 @@ static void removeRedundantMsgs(PathPiec
           break;
         
         if (PathDiagnosticEventPiece *nextEvent =
-            dyn_cast<PathDiagnosticEventPiece>(path.front())) {
+            dyn_cast<PathDiagnosticEventPiece>(path.front().getPtr())) {
           PathDiagnosticEventPiece *event =
             cast<PathDiagnosticEventPiece>(piece);
           // Check to see if we should keep one of the two pieces.  If we
@@ -210,10 +210,10 @@ bool BugReporter::RemoveUnneededCalls(Pa
 static void adjustCallLocations(PathPieces &Pieces,
                                 PathDiagnosticLocation *LastCallLocation = 0) {
   for (PathPieces::iterator I = Pieces.begin(), E = Pieces.end(); I != E; ++I) {
-    PathDiagnosticCallPiece *Call = dyn_cast<PathDiagnosticCallPiece>(&*I);
+    PathDiagnosticCallPiece *Call = dyn_cast<PathDiagnosticCallPiece>(*I);
 
     if (!Call) {
-      assert(I->getLocation().asLocation().isValid());
+      assert((*I)->getLocation().asLocation().isValid());
       continue;
     }
 
@@ -946,7 +946,7 @@ public:
       // If the PathDiagnostic already has pieces, add the enclosing statement
       // of the first piece as a context as well.
       if (!PD.path.empty()) {
-        PrevLoc = PD.path.begin()->getLocation();
+        PrevLoc = (*PD.path.begin())->getLocation();
 
         if (const Stmt *S = PrevLoc.asStmt())
           addExtendedContext(PDB.getEnclosingStmtLocation(S).asStmt());
@@ -1987,10 +1987,10 @@ static void CompactPathDiagnostic(PathPi
   MacroStackTy MacroStack;
   PiecesTy Pieces;
 
-  for (PathPieces::iterator I = path.begin(), E = path.end();
+  for (PathPieces::const_iterator I = path.begin(), E = path.end();
        I!=E; ++I) {
     
-    PathDiagnosticPiece *piece = &*I;
+    PathDiagnosticPiece *piece = I->getPtr();
 
     // Recursively compact calls.
     if (PathDiagnosticCallPiece *call=dyn_cast<PathDiagnosticCallPiece>(piece)){

Modified: cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp?rev=180746&r1=180745&r2=180746&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp Mon Apr 29 18:12:59 2013
@@ -126,10 +126,10 @@ void HTMLDiagnostics::ReportDiag(const P
 
   // The path as already been prechecked that all parts of the path are
   // from the same file and that it is non-empty.
-  const SourceManager &SMgr = path.begin()->getLocation().getManager();
+  const SourceManager &SMgr = (*path.begin())->getLocation().getManager();
   assert(!path.empty());
   FileID FID =
-    path.begin()->getLocation().asLocation().getExpansionLoc().getFileID();
+    (*path.begin())->getLocation().asLocation().getExpansionLoc().getFileID();
   assert(!FID.isInvalid());
 
   // Create a new rewriter to generate HTML.
@@ -139,10 +139,10 @@ void HTMLDiagnostics::ReportDiag(const P
   unsigned n = path.size();
   unsigned max = n;
 
-  for (PathPieces::reverse_iterator I = path.rbegin(), 
+  for (PathPieces::const_reverse_iterator I = path.rbegin(), 
        E = path.rend();
         I != E; ++I, --n)
-    HandlePiece(R, FID, *I, n, max);
+    HandlePiece(R, FID, **I, n, max);
 
   // Add line numbers, header, footer, etc.
 
@@ -185,9 +185,9 @@ void HTMLDiagnostics::ReportDiag(const P
       << html::EscapeText(Entry->getName())
       << "</td></tr>\n<tr><td class=\"rowname\">Location:</td><td>"
          "<a href=\"#EndPath\">line "
-      << path.rbegin()->getLocation().asLocation().getExpansionLineNumber()
+      << (*path.rbegin())->getLocation().asLocation().getExpansionLineNumber()
       << ", column "
-      << path.rbegin()->getLocation().asLocation().getExpansionColumnNumber()
+      << (*path.rbegin())->getLocation().asLocation().getExpansionColumnNumber()
       << "</a></td></tr>\n"
          "<tr><td class=\"rowname\">Description:</td><td>"
       << D.getVerboseDescription() << "</td></tr>\n";
@@ -503,16 +503,16 @@ unsigned HTMLDiagnostics::ProcessMacroPi
                                             const PathDiagnosticMacroPiece& P,
                                             unsigned num) {
 
-  for (PathPieces::iterator I = P.subPieces.begin(), E=P.subPieces.end();
+  for (PathPieces::const_iterator I = P.subPieces.begin(), E=P.subPieces.end();
         I!=E; ++I) {
 
     if (const PathDiagnosticMacroPiece *MP =
-          dyn_cast<PathDiagnosticMacroPiece>(&*I)) {
+          dyn_cast<PathDiagnosticMacroPiece>(*I)) {
       num = ProcessMacroPiece(os, *MP, num);
       continue;
     }
 
-    if (PathDiagnosticEventPiece *EP = dyn_cast<PathDiagnosticEventPiece>(&*I)){
+    if (PathDiagnosticEventPiece *EP = dyn_cast<PathDiagnosticEventPiece>(*I)) {
       os << "<div class=\"msg msgEvent\" style=\"width:94%; "
             "margin-left:5px\">"
             "<table class=\"msgT\"><tr>"

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=180746&r1=180745&r2=180746&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Mon Apr 29 18:12:59 2013
@@ -28,11 +28,11 @@ using namespace clang;
 using namespace ento;
 
 bool PathDiagnosticMacroPiece::containsEvent() const {
-  for (PathPieces::iterator I = subPieces.begin(), E = subPieces.end();
+  for (PathPieces::const_iterator I = subPieces.begin(), E = subPieces.end();
        I!=E; ++I) {
     if (isa<PathDiagnosticEventPiece>(*I))
       return true;
-    if (PathDiagnosticMacroPiece *MP = dyn_cast<PathDiagnosticMacroPiece>(&*I))
+    if (PathDiagnosticMacroPiece *MP = dyn_cast<PathDiagnosticMacroPiece>(*I))
       if (MP->containsEvent())
         return true;
   }
@@ -59,21 +59,13 @@ PathDiagnosticCallPiece::~PathDiagnostic
 PathDiagnosticControlFlowPiece::~PathDiagnosticControlFlowPiece() {}
 PathDiagnosticMacroPiece::~PathDiagnosticMacroPiece() {}
 
-PathPieces &PathPieces::operator=(const PathPieces & X) {
-  clear();
-  for (llvm::ilist<Node>::const_iterator I = X.L.begin(), E = X.L.end();
-       I != E; ++I) {
-    L.push_back(new Node(&*(I->Data)));
-  }
-  return *this;
-}
 
 PathPieces::~PathPieces() {}
 
 void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current,
                            bool ShouldFlattenMacros) const {
-  for (PathPieces::iterator I = begin(), E = end(); I != E; ++I) {
-    PathDiagnosticPiece *Piece = &*I;
+  for (PathPieces::const_iterator I = begin(), E = end(); I != E; ++I) {
+    PathDiagnosticPiece *Piece = I->getPtr();
 
     switch (Piece->getKind()) {
     case PathDiagnosticPiece::Call: {
@@ -153,7 +145,7 @@ void PathDiagnosticConsumer::HandlePathD
   if (!supportsCrossFileDiagnostics()) {
     // Verify that the entire path is from the same FileID.
     FileID FID;
-    const SourceManager &SMgr = D->path.begin()->getLocation().getManager();
+    const SourceManager &SMgr = (*D->path.begin())->getLocation().getManager();
     SmallVector<const PathPieces *, 5> WorkList;
     WorkList.push_back(&D->path);
 
@@ -161,9 +153,9 @@ void PathDiagnosticConsumer::HandlePathD
       const PathPieces &path = *WorkList.back();
       WorkList.pop_back();
 
-      for (PathPieces::iterator I = path.begin(), E = path.end();
+      for (PathPieces::const_iterator I = path.begin(), E = path.end();
            I != E; ++I) {
-        const PathDiagnosticPiece *piece = &*I;
+        const PathDiagnosticPiece *piece = I->getPtr();
         FullSourceLoc L = piece->getLocation().asLocation().getExpansionLoc();
       
         if (FID.isInvalid()) {
@@ -306,11 +298,11 @@ static Optional<bool> comparePath(const
   if (X.size() != Y.size())
     return X.size() < Y.size();
 
-  PathPieces::iterator X_I = X.begin(), X_end = X.end();
-  PathPieces::iterator Y_I = Y.begin(), Y_end = Y.end();
+  PathPieces::const_iterator X_I = X.begin(), X_end = X.end();
+  PathPieces::const_iterator Y_I = Y.begin(), Y_end = Y.end();
 
   for ( ; X_I != X_end && Y_I != Y_end; ++X_I, ++Y_I) {
-    Optional<bool> b = comparePiece(*X_I, *Y_I);
+    Optional<bool> b = comparePiece(**X_I, **Y_I);
     if (b.hasValue())
       return b.getValue();
   }
@@ -962,9 +954,9 @@ PathDiagnosticCallPiece::getCallExitEven
 }
 
 static void compute_path_size(const PathPieces &pieces, unsigned &size) {
-  for (PathPieces::iterator it = pieces.begin(),
+  for (PathPieces::const_iterator it = pieces.begin(),
                                   et = pieces.end(); it != et; ++it) {
-    const PathDiagnosticPiece *piece = &*it;
+    const PathDiagnosticPiece *piece = it->getPtr();
     if (const PathDiagnosticCallPiece *cp = 
         dyn_cast<PathDiagnosticCallPiece>(piece)) {
       compute_path_size(cp->path, size);
@@ -1006,9 +998,9 @@ void PathDiagnosticPiece::Profile(llvm::
 
 void PathDiagnosticCallPiece::Profile(llvm::FoldingSetNodeID &ID) const {
   PathDiagnosticPiece::Profile(ID);
-  for (PathPieces::iterator it = path.begin(), 
+  for (PathPieces::const_iterator it = path.begin(), 
        et = path.end(); it != et; ++it) {
-    ID.Add(*it);
+    ID.Add(**it);
   }
 }
 
@@ -1025,9 +1017,9 @@ void PathDiagnosticControlFlowPiece::Pro
 
 void PathDiagnosticMacroPiece::Profile(llvm::FoldingSetNodeID &ID) const {
   PathDiagnosticSpotPiece::Profile(ID);
-  for (PathPieces::iterator I = subPieces.begin(), E = subPieces.end();
+  for (PathPieces::const_iterator I = subPieces.begin(), E = subPieces.end();
        I != E; ++I)
-    ID.Add(*I);
+    ID.Add(**I);
 }
 
 void PathDiagnostic::Profile(llvm::FoldingSetNodeID &ID) const {
@@ -1039,8 +1031,8 @@ void PathDiagnostic::Profile(llvm::Foldi
 
 void PathDiagnostic::FullProfile(llvm::FoldingSetNodeID &ID) const {
   Profile(ID);
-  for (PathPieces::iterator I = path.begin(), E = path.end(); I != E; ++I)
-    ID.Add(*I);
+  for (PathPieces::const_iterator I = path.begin(), E = path.end(); I != E; ++I)
+    ID.Add(**I);
   for (meta_iterator I = meta_begin(), E = meta_end(); I != E; ++I)
     ID.AddString(*I);
 }

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp?rev=180746&r1=180745&r2=180746&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp Mon Apr 29 18:12:59 2013
@@ -294,8 +294,8 @@ static void ReportCall(raw_ostream &o,
     ReportPiece(o, *callEnterWithinCaller, FM, SM, LangOpts,
                 indent, depth, true);
   
-  for (PathPieces::iterator I = P.path.begin(), E = P.path.end();I!=E;++I)
-    ReportPiece(o, *I, FM, SM, LangOpts, indent, depth, true);
+  for (PathPieces::const_iterator I = P.path.begin(), E = P.path.end();I!=E;++I)
+    ReportPiece(o, **I, FM, SM, LangOpts, indent, depth, true);
 
   --depth;
   
@@ -313,9 +313,9 @@ static void ReportMacro(raw_ostream &o,
                         unsigned indent,
                         unsigned depth) {
 
-  for (PathPieces::iterator I = P.subPieces.begin(), E=P.subPieces.end();
+  for (PathPieces::const_iterator I = P.subPieces.begin(), E=P.subPieces.end();
        I!=E; ++I) {
-    ReportPiece(o, *I, FM, SM, LangOpts, indent, depth, false);
+    ReportPiece(o, **I, FM, SM, LangOpts, indent, depth, false);
   }
 }
 
@@ -363,7 +363,7 @@ void PlistDiagnostics::FlushDiagnosticsI
   const SourceManager* SM = 0;
 
   if (!Diags.empty())
-    SM = &(*(*Diags.begin())->path.begin()).getLocation().getManager();
+    SM = &(*(*Diags.begin())->path.begin())->getLocation().getManager();
 
   
   for (std::vector<const PathDiagnostic*>::iterator DI = Diags.begin(),
@@ -378,9 +378,9 @@ void PlistDiagnostics::FlushDiagnosticsI
       const PathPieces &path = *WorkList.back();
       WorkList.pop_back();
     
-      for (PathPieces::iterator I = path.begin(), E = path.end();
+      for (PathPieces::const_iterator I = path.begin(), E = path.end();
            I!=E; ++I) {
-        const PathDiagnosticPiece *piece = &*I;
+        const PathDiagnosticPiece *piece = I->getPtr();
         AddFID(FM, Fids, SM, piece->getLocation().asLocation());
         ArrayRef<SourceRange> Ranges = piece->getRanges();
         for (ArrayRef<SourceRange>::iterator I = Ranges.begin(),
@@ -450,9 +450,9 @@ void PlistDiagnostics::FlushDiagnosticsI
 
     o << "   <array>\n";
 
-    for (PathPieces::iterator I = D->path.begin(), E = D->path.end(); 
+    for (PathPieces::const_iterator I = D->path.begin(), E = D->path.end(); 
          I != E; ++I)
-      ReportDiag(o, *I, FM, *SM, LangOpts);
+      ReportDiag(o, **I, FM, *SM, LangOpts);
 
     o << "   </array>\n";
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp?rev=180746&r1=180745&r2=180746&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/TextPathDiagnostics.cpp Mon Apr 29 18:12:59 2013
@@ -61,12 +61,12 @@ void TextPathDiagnostics::FlushDiagnosti
     const PathDiagnostic *D = *it;
 
     PathPieces FlatPath = D->path.flatten(/*ShouldFlattenMacros=*/true);
-    for (PathPieces::iterator I = FlatPath.begin(), E = FlatPath.end(); 
+    for (PathPieces::const_iterator I = FlatPath.begin(), E = FlatPath.end(); 
          I != E; ++I) {
       unsigned diagID =
         Diag.getDiagnosticIDs()->getCustomDiagID(DiagnosticIDs::Note,
-                                                 I->getString());
-      Diag.Report(I->getLocation().asLocation(), diagID);
+                                                 (*I)->getString());
+      Diag.Report((*I)->getLocation().asLocation(), diagID);
     }
   }
 }





More information about the cfe-commits mailing list