[cfe-commits] r52553 - in /cfe/trunk: include/clang/AST/ParentMap.h lib/AST/ParentMap.cpp

[Chris Lattner]

On Jun 20, 2008, at 2:40 PM, Ted Kremenek wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=52553&view=rev
> Log:
> Added ParentMap, a class to represent a lazily constructed mapping  
> from child to parents.

Spiffy.

> +namespace clang {
> +class Stmt;
> +
> +class ParentMap {

Please add a big doxygen comment above the class explaining what this  
is.

>
> +  void* Impl;
> +public:
> +  ParentMap(Stmt* ASTRoot);
> +  ~ParentMap();
> +
> +  Stmt* getParent(Stmt*) const;
> +
> +  bool hasParent(Stmt* S) const {
> +    return !getParent(S);
> +  }
> +
> +  bool isSubExpr(Stmt *S) const;
> +};

Very nice and minimal interface, I like it.  Please document what  
'isSubExpr' does.  even from the implementation it isn't clear to me.


> +++ cfe/trunk/lib/AST/ParentMap.cpp Fri Jun 20 16:40:36 2008
> @@ -0,0 +1,54 @@
> +//===--- ParentMap.cpp - Mappings from Stmts to their Parents ---*-  
> C++ -*-===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open  
> Source
> +// License. See LICENSE.TXT for details.
> +//
> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +//
> +//  This file defines the ParentMap class.

s/defines/implements

> +static void BuildParentMap(MapTy& M, Stmt* S) {
> +  for (Stmt::child_iterator I=S->child_begin(), E=S->child_end(); I! 
> =E; ++I)
> +    if (*I) {
> +      M[*I] = S;
> +      BuildParentMap(M, *I);
> +    }
> +}
> +
> +ParentMap::ParentMap(Stmt* S) : Impl(0) {
> +  if (S) {
> +    MapTy *M = new MapTy();
> +    BuildParentMap(*M, S);
> +    Impl = M;
> +  }
> +}

When would it ever be useful to pass in a null S here?  Should the if  
be an assert?

-Chris