r242608 - [AST] Cleanup ExprIterator.
Alexandros Tzannes
atzannes at illinois.edu
Tue Jul 28 01:09:36 PDT 2015
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 at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list