Hi Benjamin,
is there an advantage to making ExprIterator and ConstExprIterator protected in Stmt?

I have some methods that takes arguments of type ExpIterator and used to work for several different classes derived from Stmt (e.g., CXXConstructExpr, CallExpr, ...). Now that Stmt::ExprIterator is protected the code does not compile anymore. (E.g., void foo(ExprIterator ArgI, ExprIterator ArgE) which basically factors out a loop over expressions).

Right now, I think my best bet is to make these methods generic (template for type EXPR_ITERATOR) and have it specialized for the different public typedefs of ExprIterator (CXXConstructExpr::arg_iterator, CallExpr::arg_iterator). Is there a better approach? I'm also thinking of making ExprIterator public in the private branch of Clang that I'm working on.

Thanks in advance,
Alex

On 07/18/2015 05:35 PM, Benjamin Kramer wrote:
Author: d0k
Date: Sat Jul 18 09:35:53 2015
New Revision: 242608

URL: http://llvm.org/viewvc/llvm-project?rev=242608&view=rev
Log:
[AST] Cleanup ExprIterator.

- Make it a proper random access iterator with a little help from 
iterator_adaptor_base
- Clean up users of magic dereferencing. The iterator should behave like an 
Expr **.
- Make it an implementation detail of Stmt. This allows inlining of the 
assertions.

Modified:
     cfe/trunk/include/clang/AST/Stmt.h
     cfe/trunk/lib/AST/Expr.cpp
     cfe/trunk/lib/AST/StmtPrinter.cpp
     cfe/trunk/lib/CodeGen/CGCall.cpp
     cfe/trunk/lib/CodeGen/CGClass.cpp
     cfe/trunk/lib/CodeGen/CGExprCXX.cpp
     cfe/trunk/lib/CodeGen/CodeGenFunction.h
     cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=242608&r1=242607&r2=242608&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Sat Jul 18 09:35:53 2015
@@ -22,6 +22,7 @@
  #include "clang/Basic/SourceLocation.h"
  #include "llvm/ADT/ArrayRef.h"
  #include "llvm/ADT/PointerIntPair.h"
+#include "llvm/ADT/iterator.h"
  #include "llvm/Support/Compiler.h"
  #include "llvm/Support/ErrorHandling.h"
  #include <string>
@@ -49,57 +50,6 @@ namespace clang {
    class Token;
    class VarDecl;
- //===--------------------------------------------------------------------===//
-  // ExprIterator - Iterators for iterating over Stmt* arrays that contain
-  //  only Expr*.  This is needed because AST nodes use Stmt* arrays to store
-  //  references to children (to be compatible with StmtIterator).
-  
//===--------------------------------------------------------------------===//
-
-  class Stmt;
-  class Expr;
-
-  class ExprIterator : public std::iterator<std::forward_iterator_tag,
-                                            Expr *&, ptrdiff_t,
-                                            Expr *&, Expr *&> {
-    Stmt** I;
-  public:
-    ExprIterator(Stmt** i) : I(i) {}
-    ExprIterator() : I(nullptr) {}
-    ExprIterator& operator++() { ++I; return *this; }
-    ExprIterator operator-(size_t i) { return I-i; }
-    ExprIterator operator+(size_t i) { return I+i; }
-    Expr* operator[](size_t idx);
-    // FIXME: Verify that this will correctly return a signed distance.
-    signed operator-(const ExprIterator& R) const { return I - R.I; }
-    Expr* operator*() const;
-    Expr* operator->() const;
-    bool operator==(const ExprIterator& R) const { return I == R.I; }
-    bool operator!=(const ExprIterator& R) const { return I != R.I; }
-    bool operator>(const ExprIterator& R) const { return I > R.I; }
-    bool operator>=(const ExprIterator& R) const { return I >= R.I; }
-  };
-
-  class ConstExprIterator : public std::iterator<std::forward_iterator_tag,
-                                                 const Expr *&, ptrdiff_t,
-                                                 const Expr *&,
-                                                 const Expr *&> {
-    const Stmt * const *I;
-  public:
-    ConstExprIterator(const Stmt * const *i) : I(i) {}
-    ConstExprIterator() : I(nullptr) {}
-    ConstExprIterator& operator++() { ++I; return *this; }
-    ConstExprIterator operator+(size_t i) const { return I+i; }
-    ConstExprIterator operator-(size_t i) const { return I-i; }
-    const Expr * operator[](size_t idx) const;
-    signed operator-(const ConstExprIterator& R) const { return I - R.I; }
-    const Expr * operator*() const;
-    const Expr * operator->() const;
-    bool operator==(const ConstExprIterator& R) const { return I == R.I; }
-    bool operator!=(const ConstExprIterator& R) const { return I != R.I; }
-    bool operator>(const ConstExprIterator& R) const { return I > R.I; }
-    bool operator>=(const ConstExprIterator& R) const { return I >= R.I; }
-  };
-
  
//===----------------------------------------------------------------------===//
  // AST classes for statements.
  
//===----------------------------------------------------------------------===//
@@ -337,6 +287,39 @@ public:
    /// de-serialization).
    struct EmptyShell { };
+protected:
+  /// Iterator for iterating over Stmt * arrays that contain only Expr *
+  ///
+  /// This is needed because AST nodes use Stmt* arrays to store
+  /// references to children (to be compatible with StmtIterator).
+  struct ExprIterator
+      : llvm::iterator_adaptor_base<ExprIterator, Stmt **,
+                                    std::random_access_iterator_tag, Expr *> {
+    ExprIterator() : iterator_adaptor_base(nullptr) {}
+    ExprIterator(Stmt **I) : iterator_adaptor_base(I) {}
+
+    reference operator*() const {
+      assert((*I)->getStmtClass() >= firstExprConstant &&
+             (*I)->getStmtClass() <= lastExprConstant);
+      return *reinterpret_cast<Expr **>(I);
+    }
+  };
+
+  /// Const iterator for iterating over Stmt * arrays that contain only Expr *
+  struct ConstExprIterator
+      : llvm::iterator_adaptor_base<ConstExprIterator, const Stmt *const *,
+                                    std::random_access_iterator_tag,
+                                    const Expr *const> {
+    ConstExprIterator() : iterator_adaptor_base(nullptr) {}
+    ConstExprIterator(const Stmt *const *I) : iterator_adaptor_base(I) {}
+
+    reference operator*() const {
+      assert((*I)->getStmtClass() >= firstExprConstant &&
+             (*I)->getStmtClass() <= lastExprConstant);
+      return *reinterpret_cast<const Expr *const *>(I);
+    }
+  };
+
  private:
    /// \brief Whether statistic collection is enabled.
    static bool StatisticsEnabled;

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=242608&r1=242607&r2=242608&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Sat Jul 18 09:35:53 2015
@@ -4163,19 +4163,6 @@ PseudoObjectExpr::PseudoObjectExpr(QualT
  }
//===----------------------------------------------------------------------===//
-//  ExprIterator.
-//===----------------------------------------------------------------------===//
-
-Expr* ExprIterator::operator[](size_t idx) { return cast<Expr>(I[idx]); }
-Expr* ExprIterator::operator*() const { return cast<Expr>(*I); }
-Expr* ExprIterator::operator->() const { return cast<Expr>(*I); }
-const Expr* ConstExprIterator::operator[](size_t idx) const {
-  return cast<Expr>(I[idx]);
-}
-const Expr* ConstExprIterator::operator*() const { return cast<Expr>(*I); }
-const Expr* ConstExprIterator::operator->() const { return cast<Expr>(*I); }
-
-//===----------------------------------------------------------------------===//
  //  Child Iterators for iterating over subexpressions/substatements
  
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/lib/AST/StmtPrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/StmtPrinter.cpp?rev=242608&r1=242607&r2=242608&view=diff
==============================================================================
--- cfe/trunk/lib/AST/StmtPrinter.cpp (original)
+++ cfe/trunk/lib/AST/StmtPrinter.cpp Sat Jul 18 09:35:53 2015
@@ -1768,7 +1768,7 @@ void StmtPrinter::VisitCXXTemporaryObjec
    for (CXXTemporaryObjectExpr::arg_iterator Arg = Node->arg_begin(),
                                           ArgEnd = Node->arg_end();
         Arg != ArgEnd; ++Arg) {
-    if (Arg->isDefaultArgument())
+    if ((*Arg)->isDefaultArgument())
        break;
      if (Arg != Node->arg_begin())
        OS << ", ";

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=242608&r1=242607&r2=242608&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Sat Jul 18 09:35:53 2015
@@ -2829,7 +2829,7 @@ void CodeGenFunction::EmitCallArgs(CallA
      for (int I = ArgTypes.size() - 1; I >= 0; --I) {
        CallExpr::const_arg_iterator Arg = ArgBeg + I;
        EmitCallArg(Args, *Arg, ArgTypes[I]);
-      EmitNonNullArgCheck(Args.back().RV, ArgTypes[I], Arg->getExprLoc(),
+      EmitNonNullArgCheck(Args.back().RV, ArgTypes[I], (*Arg)->getExprLoc(),
                            CalleeDecl, ParamsToSkip + I);
      }
@@ -2843,7 +2843,7 @@ void CodeGenFunction::EmitCallArgs(CallA
      CallExpr::const_arg_iterator Arg = ArgBeg + I;
      assert(Arg != ArgEnd);
      EmitCallArg(Args, *Arg, ArgTypes[I]);
-    EmitNonNullArgCheck(Args.back().RV, ArgTypes[I], Arg->getExprLoc(),
+    EmitNonNullArgCheck(Args.back().RV, ArgTypes[I], (*Arg)->getExprLoc(),
                          CalleeDecl, ParamsToSkip + I);
    }
  }

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=242608&r1=242607&r2=242608&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Sat Jul 18 09:35:53 2015
@@ -1832,7 +1832,7 @@ CodeGenFunction::EmitSynthesizedCXXCopyC
             "trivial 1-arg ctor not a copy/move ctor");
      EmitAggregateCopyCtor(This, Src,
                            getContext().getTypeDeclType(D->getParent()),
-                          E->arg_begin()->getType());
+                          (*E->arg_begin())->getType());
      return;
    }
    llvm::Value *Callee = CGM.getAddrOfCXXStructor(D, StructorType::Complete);

Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=242608&r1=242607&r2=242608&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Sat Jul 18 09:35:53 2015
@@ -196,7 +196,7 @@ RValue CodeGenFunction::EmitCXXMemberOrO
          // Trivial move and copy ctor are the same.
          assert(CE->getNumArgs() == 1 && "unexpected argcount for trivial 
ctor");
          llvm::Value *RHS = EmitLValue(*CE->arg_begin()).getAddress();
-        EmitAggregateCopy(This, RHS, CE->arg_begin()->getType());
+        EmitAggregateCopy(This, RHS, (*CE->arg_begin())->getType());
          return RValue::get(This);
        }
        llvm_unreachable("unknown trivial member function");
@@ -1089,8 +1089,7 @@ RValue CodeGenFunction::EmitBuiltinNewDe
                                                   bool IsDelete) {
    CallArgList Args;
    const Stmt *ArgS = Arg;
-  EmitCallArgs(Args, *Type->param_type_begin(),
-               ConstExprIterator(&ArgS), ConstExprIterator(&ArgS + 1));
+  EmitCallArgs(Args, *Type->param_type_begin(), &ArgS, &ArgS + 1);
    // Find the allocation or deallocation function that we're calling.
    ASTContext &Ctx = getContext();
    DeclarationName Name = Ctx.DeclarationNames

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=242608&r1=242607&r2=242608&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Sat Jul 18 09:35:53 2015
@@ -2992,7 +2992,7 @@ public:
                           .getCanonicalType((*I).getNonReferenceType())
                           .getTypePtr() ==
                       getContext()
-                         .getCanonicalType(Arg->getType())
+                         .getCanonicalType((*Arg)->getType())
                           .getTypePtr())) &&
                 "type mismatch in call argument!");
          ArgTypes.push_back(*I);

Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=242608&r1=242607&r2=242608&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Sat Jul 18 09:35:53 2015
@@ -3803,9 +3803,8 @@ MicrosoftCXXABI::getAddrOfCXXCtorClosure
    CodeGenFunction::RunCleanupsScope Cleanups(CGF);
const auto *FPT = CD->getType()->castAs<FunctionProtoType>();
-  ConstExprIterator ArgBegin(ArgVec.data()),
-      ArgEnd(ArgVec.data() + ArgVec.size());
-  CGF.EmitCallArgs(Args, FPT, ArgBegin, ArgEnd, CD, IsCopy ? 1 : 0);
+  CGF.EmitCallArgs(Args, FPT, &*ArgVec.begin(), &*ArgVec.end(), CD,
+                   IsCopy ? 1 : 0);
// Insert any ABI-specific implicit constructor arguments.
    unsigned ExtraArgs = addImplicitConstructorArgs(CGF, CD, Ctor_Complete,


_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to