From a88cfbfac9bbcbb9858f048d6d73a48711d8e93d Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Sat, 12 Dec 2009 18:16:41 +0000 Subject: [PATCH] Rework the way we handle template instantiation for implicitly-generated AST nodes. We previously built instantiated nodes for each of these AST nodes, then passed them on to Sema, which was not prepared to see already-type-checked nodes (see PR5755). In some places, we had ugly workarounds to try to avoid re-type-checking (e.g., in VarDecl initializer instantiation). Now, we skip implicitly-generated nodes when performing instantiation, preferring instead to build just the AST nodes that directly reflect what was written in the source code. This has several advantages: - We don't need to instantiate anything that doesn't have a direct correlation to the source code, so we can have better location information. - Semantic analysis sees the same thing at template instantiation time that it would see for a non-template. - At least one ugly hack (VarDecl initializers) goes away. Fixes PR5755. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@91218 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/ExprCXX.h | 1 + lib/Sema/SemaTemplateInstantiateDecl.cpp | 53 +++++++++++++---- lib/Sema/TreeTransform.h | 72 ++++++++++++------------ test/SemaTemplate/instantiate-expr-4.cpp | 12 ++++ 4 files changed, 91 insertions(+), 47 deletions(-) diff --git a/include/clang/AST/ExprCXX.h b/include/clang/AST/ExprCXX.h index 66f4b5d2b5..00ea202abd 100644 --- a/include/clang/AST/ExprCXX.h +++ b/include/clang/AST/ExprCXX.h @@ -527,6 +527,7 @@ public: const_arg_iterator arg_begin() const { return Args; } const_arg_iterator arg_end() const { return Args + NumArgs; } + Expr **getArgs() const { return reinterpret_cast(Args); } unsigned getNumArgs() const { return NumArgs; } /// getArg - Return the specified argument. diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp index 07e97117cb..4512bcde16 100644 --- a/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -16,6 +16,7 @@ #include "clang/AST/DeclTemplate.h" #include "clang/AST/DeclVisitor.h" #include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" #include "clang/Basic/PrettyStackTrace.h" #include "clang/Lex/Preprocessor.h" @@ -204,16 +205,6 @@ Decl *TemplateDeclInstantiator::VisitVarDecl(VarDecl *D) { = SemaRef.SubstExpr(D->getInit(), TemplateArgs); if (Init.isInvalid()) Var->setInvalidDecl(); - else if (!D->getType()->isDependentType() && - !D->getInit()->isTypeDependent() && - !D->getInit()->isValueDependent()) { - // If neither the declaration's type nor its initializer are dependent, - // we don't want to redo all the checking, especially since the - // initializer might have been wrapped by a CXXConstructExpr since we did - // it the first time. - Var->setType(D->getType()); - Var->setInit(SemaRef.Context, Init.takeAs()); - } else if (ParenListExpr *PLE = dyn_cast((Expr *)Init.get())) { // FIXME: We're faking all of the comma locations, which is suboptimal. // Do we even need these comma locations? @@ -239,7 +230,47 @@ Decl *TemplateDeclInstantiator::VisitVarDecl(VarDecl *D) { // When Init is destroyed, it will destroy the instantiated ParenListExpr; // we've explicitly retained all of its subexpressions already. - } else + } else if (CXXConstructExpr *Construct + = dyn_cast((Expr *)Init.get())) { + // We build CXXConstructExpr nodes to capture the implicit + // construction of objects. Rip apart the CXXConstructExpr to + // pass its pieces down to the appropriate initialization + // function. + if (D->hasCXXDirectInitializer()) { + // FIXME: Poor source location information + SourceLocation FakeLParenLoc = + SemaRef.PP.getLocForEndOfToken(D->getLocation()); + SourceLocation FakeRParenLoc = FakeLParenLoc; + llvm::SmallVector FakeCommaLocs; + if (Construct->getNumArgs() > 0) { + FakeRParenLoc + = SemaRef.PP.getLocForEndOfToken( + Construct->getArg(Construct->getNumArgs() - 1)->getLocEnd()); + + FakeCommaLocs.reserve(Construct->getNumArgs() - 1); + for (unsigned I = 0, N = Construct->getNumArgs() - 1; I != N; ++I) { + Expr *E = Construct->getArg(I)->Retain(); + FakeCommaLocs.push_back( + SemaRef.PP.getLocForEndOfToken(E->getLocEnd())); + } + Construct->getArg(Construct->getNumArgs() - 1)->Retain(); + } + + SemaRef.AddCXXDirectInitializerToDecl(Sema::DeclPtrTy::make(Var), + FakeLParenLoc, + Sema::MultiExprArg(SemaRef, + (void **)Construct->getArgs(), + Construct->getNumArgs()), + FakeCommaLocs.data(), + FakeRParenLoc); + + } else if (Construct->getNumArgs() >= 1) { + SemaRef.AddInitializerToDecl(Sema::DeclPtrTy::make(Var), + SemaRef.Owned(Construct->getArg(0)->Retain()), + false); + } else + SemaRef.ActOnUninitializedDecl(Sema::DeclPtrTy::make(Var), false); + } else SemaRef.AddInitializerToDecl(Sema::DeclPtrTy::make(Var), move(Init), D->hasCXXDirectInitializer()); SemaRef.PopExpressionEvaluationContext(); diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h index 643489fa37..f8e8f947be 100644 --- a/lib/Sema/TreeTransform.h +++ b/lib/Sema/TreeTransform.h @@ -993,19 +993,6 @@ public: move(LHS), move(RHS)); } - /// \brief Build a new implicit cast expression. - /// - /// By default, builds a new implicit cast without any semantic analysis. - /// Subclasses may override this routine to provide different behavior. - OwningExprResult RebuildImplicitCastExpr(QualType T, CastExpr::CastKind Kind, - ExprArg SubExpr, bool isLvalue) { - ImplicitCastExpr *ICE - = new (getSema().Context) ImplicitCastExpr(T, Kind, - (Expr *)SubExpr.release(), - isLvalue); - return getSema().Owned(ICE); - } - /// \brief Build a new C-style cast expression. /// /// By default, performs semantic analysis to build the new expression. @@ -3779,29 +3766,39 @@ TreeTransform::TransformConditionalOperator(ConditionalOperator *E) { move(RHS)); } +/// \brief Given a cast expression, extract the subexpression of the +/// cast, looking through intermediate AST nodes that were generated +/// as part of type checking. +static Expr *getCastSubExprAsWritten(CastExpr *E) { + Expr *SubExpr = 0; + do { + SubExpr = E->getSubExpr(); + + // Temporaries will be re-bound when rebuilding the original cast + // expression. + if (CXXBindTemporaryExpr *Binder = dyn_cast(SubExpr)) + SubExpr = Binder->getSubExpr(); + + // Conversions by constructor and conversion functions have a + // subexpression describing the call; strip it off. + if (E->getCastKind() == CastExpr::CK_ConstructorConversion) + SubExpr = cast(SubExpr)->getArg(0); + else if (E->getCastKind() == CastExpr::CK_UserDefinedConversion) + SubExpr = cast(SubExpr)->getImplicitObjectArgument(); + + // If the subexpression we're left with is an implicit cast, look + // through that, too. + } while ((E = dyn_cast(SubExpr))); + + return SubExpr; +} + template Sema::OwningExprResult TreeTransform::TransformImplicitCastExpr(ImplicitCastExpr *E) { - TemporaryBase Rebase(*this, E->getLocStart(), DeclarationName()); - - // FIXME: Will we ever have type information here? It seems like we won't, - // so do we even need to transform the type? - QualType T = getDerived().TransformType(E->getType()); - if (T.isNull()) - return SemaRef.ExprError(); - - OwningExprResult SubExpr = getDerived().TransformExpr(E->getSubExpr()); - if (SubExpr.isInvalid()) - return SemaRef.ExprError(); - - if (!getDerived().AlwaysRebuild() && - T == E->getType() && - SubExpr.get() == E->getSubExpr()) - return SemaRef.Owned(E->Retain()); - - return getDerived().RebuildImplicitCastExpr(T, E->getCastKind(), - move(SubExpr), - E->isLvalueCast()); + // Implicit casts are eliminated during transformation, since they + // will be recomputed by semantic analysis after transformation. + return getDerived().TransformExpr(getCastSubExprAsWritten(E)); } template @@ -3826,7 +3823,8 @@ TreeTransform::TransformCStyleCastExpr(CStyleCastExpr *E) { return SemaRef.ExprError(); } - OwningExprResult SubExpr = getDerived().TransformExpr(E->getSubExpr()); + OwningExprResult SubExpr + = getDerived().TransformExpr(getCastSubExprAsWritten(E)); if (SubExpr.isInvalid()) return SemaRef.ExprError(); @@ -4182,7 +4180,8 @@ TreeTransform::TransformCXXNamedCastExpr(CXXNamedCastExpr *E) { return SemaRef.ExprError(); } - OwningExprResult SubExpr = getDerived().TransformExpr(E->getSubExpr()); + OwningExprResult SubExpr + = getDerived().TransformExpr(getCastSubExprAsWritten(E)); if (SubExpr.isInvalid()) return SemaRef.ExprError(); @@ -4246,7 +4245,8 @@ TreeTransform::TransformCXXFunctionalCastExpr( return SemaRef.ExprError(); } - OwningExprResult SubExpr = getDerived().TransformExpr(E->getSubExpr()); + OwningExprResult SubExpr + = getDerived().TransformExpr(getCastSubExprAsWritten(E)); if (SubExpr.isInvalid()) return SemaRef.ExprError(); diff --git a/test/SemaTemplate/instantiate-expr-4.cpp b/test/SemaTemplate/instantiate-expr-4.cpp index 150faec3f2..b99ec3304c 100644 --- a/test/SemaTemplate/instantiate-expr-4.cpp +++ b/test/SemaTemplate/instantiate-expr-4.cpp @@ -96,6 +96,18 @@ template struct Delete0; template struct Delete0; template struct Delete0; // expected-note{{instantiation}} +namespace PR5755 { + template + void Foo() { + char* p = 0; + delete[] p; + } + + void Test() { + Foo(); + } +} + // --------------------------------------------------------------------- // throw expressions // ---------------------------------------------------------------------