[cfe-commits] r113700 - in /cfe/trunk: include/clang/AST/EvaluatedExprVisitor.h include/clang/Sema/Sema.h lib/Parse/ParseCXXInlineMethods.cpp lib/Parse/ParseDecl.cpp lib/Sema/SemaExpr.cpp test/SemaTemplate/default-expr-arguments.cpp
Douglas Gregor
dgregor at apple.com
Sat Sep 11 13:24:53 PDT 2010
Author: dgregor
Date: Sat Sep 11 15:24:53 2010
New Revision: 113700
URL: http://llvm.org/viewvc/llvm-project?rev=113700&view=rev
Log:
When parsing default function arguments, do not mark any declarations
used in the default function argument as "used". Instead, when we
actually use the default argument, make another pass over the
expression to mark any used declarations as "used" at that point. This
addresses two kinds of related problems:
1) We were marking some declarations "used" that shouldn't be,
because we were marking them too eagerly.
2) We were failing to mark some declarations as "used" when we
should, if the first time it was instantiated happened to be an
unevaluated context, we wouldn't mark them again at a later point.
I've also added a potentially-handy visitor class template
EvaluatedExprVisitor, which only visits the potentially-evaluated
subexpressions of an expression. I bet this would have been useful for
noexcept...
Fixes PR5810 and PR8127.
Added:
cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h (with props)
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/SemaTemplate/default-expr-arguments.cpp
Added: cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h?rev=113700&view=auto
==============================================================================
--- cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h (added)
+++ cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h Sat Sep 11 15:24:53 2010
@@ -0,0 +1,74 @@
+//===--- EvaluatedExprVisitor.h - Evaluated expression visitor --*- 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 EvaluatedExprVisitor class template, which visits
+// the potentially-evaluated subexpressions of a potentially-evaluated
+// expression.
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_CLANG_AST_EVALUATEDEXPRVISITOR_H
+#define LLVM_CLANG_AST_EVALUATEDEXPRVISITOR_H
+
+#include "clang/AST/StmtVisitor.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+
+namespace clang {
+
+class ASTContext;
+
+/// \begin Given a potentially-evaluated expression, this visitor visits all
+/// of its potentially-evaluated subexpressions, recursively.
+template<typename ImplClass>
+class EvaluatedExprVisitor : public StmtVisitor<ImplClass> {
+ ASTContext &Context;
+
+public:
+ explicit EvaluatedExprVisitor(ASTContext &Context) : Context(Context) { }
+
+ // Expressions that have no potentially-evaluated subexpressions (but may have
+ // other sub-expressions).
+ void VisitDeclRefExpr(DeclRefExpr *E) { }
+ void VisitOffsetOfExpr(OffsetOfExpr *E) { }
+ void VisitSizeOfAlignOfExpr(SizeOfAlignOfExpr *E) { }
+ void VisitBlockExpr(BlockExpr *E) { }
+ void VisitCXXUuidofExpr(CXXUuidofExpr *E) { }
+ void VisitCXXNoexceptExpr(CXXNoexceptExpr *E) { }
+
+ void VisitMemberExpr(MemberExpr *E) {
+ // Only the base matters.
+ return this->Visit(E->getBase());
+ }
+
+ void VisitChooseExpr(ChooseExpr *E) {
+ // Only the selected subexpression matters; the other one is not evaluated.
+ return this->Visit(E->getChosenSubExpr(Context));
+ }
+
+ void VisitDesignatedInitExpr(DesignatedInitExpr *E) {
+ // Only the actual initializer matters; the designators are all constant
+ // expressions.
+ return this->Visit(E->getInit());
+ }
+
+ void VisitCXXTypeidExpr(CXXTypeidExpr *E) {
+ // typeid(expression) is potentially evaluated when the argument is
+ // a glvalue of polymorphic type. (C++ 5.2.8p2-3)
+ if (!E->isTypeOperand() && E->Classify(Context).isGLValue())
+ if (const RecordType *Record
+ = E->getExprOperand()->getType()->getAs<RecordType>())
+ if (cast<CXXRecordDecl>(Record->getDecl())->isPolymorphic())
+ return this->Visit(E->getExprOperand());
+ }
+};
+
+}
+
+#endif // LLVM_CLANG_AST_EVALUATEDEXPRVISITOR_H
Propchange: cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h
------------------------------------------------------------------------------
svn:keywords = Id
Propchange: cfe/trunk/include/clang/AST/EvaluatedExprVisitor.h
------------------------------------------------------------------------------
svn:mime-type = text/plain
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=113700&r1=113699&r2=113700&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Sat Sep 11 15:24:53 2010
@@ -404,7 +404,17 @@
/// This evaluation context is used primary for the operand of the C++
/// \c typeid expression, whose argument is potentially evaluated only when
/// it is an lvalue of polymorphic class type (C++ [basic.def.odr]p2).
- PotentiallyPotentiallyEvaluated
+ PotentiallyPotentiallyEvaluated,
+
+ /// \brief The current expression is potentially evaluated, but any
+ /// declarations referenced inside that expression are only used if
+ /// in fact the current expression is used.
+ ///
+ /// This value is used when parsing default function arguments, for which
+ /// we would like to provide diagnostics (e.g., passing non-POD arguments
+ /// through varargs) but do not want to mark declarations as "referenced"
+ /// until the default argument is used.
+ PotentiallyEvaluatedIfUsed
};
/// \brief Data structure used to record current or nested
@@ -1656,6 +1666,7 @@
void MarkDeclarationReferenced(SourceLocation Loc, Decl *D);
void MarkDeclarationsReferencedInType(SourceLocation Loc, QualType T);
+ void MarkDeclarationsReferencedInExpr(Expr *E);
bool DiagRuntimeBehavior(SourceLocation Loc, const PartialDiagnostic &PD);
// Primary Expressions.
Modified: cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp?rev=113700&r1=113699&r2=113700&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp (original)
+++ cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp Sat Sep 11 15:24:53 2010
@@ -138,6 +138,11 @@
assert(Tok.is(tok::equal) && "Default argument not starting with '='");
SourceLocation EqualLoc = ConsumeToken();
+ // The argument isn't actually potentially evaluated unless it is
+ // used.
+ EnterExpressionEvaluationContext Eval(Actions,
+ Sema::PotentiallyEvaluatedIfUsed);
+
ExprResult DefArgResult(ParseAssignmentExpression());
if (DefArgResult.isInvalid())
Actions.ActOnParamDefaultArgumentError(LM.DefaultArgs[I].Param);
Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=113700&r1=113699&r2=113700&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Sat Sep 11 15:24:53 2010
@@ -3192,6 +3192,11 @@
// Consume the '='.
ConsumeToken();
+ // The argument isn't actually potentially evaluated unless it is
+ // used.
+ EnterExpressionEvaluationContext Eval(Actions,
+ Sema::PotentiallyEvaluatedIfUsed);
+
ExprResult DefArgResult(ParseAssignmentExpression());
if (DefArgResult.isInvalid()) {
Actions.ActOnParamDefaultArgumentError(Param);
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=113700&r1=113699&r2=113700&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Sat Sep 11 15:24:53 2010
@@ -19,6 +19,7 @@
#include "clang/AST/CXXInheritance.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/EvaluatedExprVisitor.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/ExprObjC.h"
@@ -3401,57 +3402,59 @@
FunctionDecl *FD,
ParmVarDecl *Param) {
if (Param->hasUnparsedDefaultArg()) {
- Diag (CallLoc,
+ Diag(CallLoc,
diag::err_use_of_default_argument_to_function_declared_later) <<
FD << cast<CXXRecordDecl>(FD->getDeclContext())->getDeclName();
Diag(UnparsedDefaultArgLocs[Param],
diag::note_default_argument_declared_here);
- } else {
- if (Param->hasUninstantiatedDefaultArg()) {
- Expr *UninstExpr = Param->getUninstantiatedDefaultArg();
+ return ExprError();
+ }
+
+ if (Param->hasUninstantiatedDefaultArg()) {
+ Expr *UninstExpr = Param->getUninstantiatedDefaultArg();
- // Instantiate the expression.
- MultiLevelTemplateArgumentList ArgList
- = getTemplateInstantiationArgs(FD, 0, /*RelativeToPrimary=*/true);
-
- std::pair<const TemplateArgument *, unsigned> Innermost
- = ArgList.getInnermost();
- InstantiatingTemplate Inst(*this, CallLoc, Param, Innermost.first,
- Innermost.second);
+ // Instantiate the expression.
+ MultiLevelTemplateArgumentList ArgList
+ = getTemplateInstantiationArgs(FD, 0, /*RelativeToPrimary=*/true);
+
+ std::pair<const TemplateArgument *, unsigned> Innermost
+ = ArgList.getInnermost();
+ InstantiatingTemplate Inst(*this, CallLoc, Param, Innermost.first,
+ Innermost.second);
- ExprResult Result = SubstExpr(UninstExpr, ArgList);
- if (Result.isInvalid())
- return ExprError();
-
- // Check the expression as an initializer for the parameter.
- InitializedEntity Entity
- = InitializedEntity::InitializeParameter(Param);
- InitializationKind Kind
- = InitializationKind::CreateCopy(Param->getLocation(),
- /*FIXME:EqualLoc*/UninstExpr->getSourceRange().getBegin());
- Expr *ResultE = Result.takeAs<Expr>();
-
- InitializationSequence InitSeq(*this, Entity, Kind, &ResultE, 1);
- Result = InitSeq.Perform(*this, Entity, Kind,
- MultiExprArg(*this, &ResultE, 1));
- if (Result.isInvalid())
- return ExprError();
+ ExprResult Result = SubstExpr(UninstExpr, ArgList);
+ if (Result.isInvalid())
+ return ExprError();
- // Build the default argument expression.
- return Owned(CXXDefaultArgExpr::Create(Context, CallLoc, Param,
- Result.takeAs<Expr>()));
- }
+ // Check the expression as an initializer for the parameter.
+ InitializedEntity Entity
+ = InitializedEntity::InitializeParameter(Param);
+ InitializationKind Kind
+ = InitializationKind::CreateCopy(Param->getLocation(),
+ /*FIXME:EqualLoc*/UninstExpr->getSourceRange().getBegin());
+ Expr *ResultE = Result.takeAs<Expr>();
+
+ InitializationSequence InitSeq(*this, Entity, Kind, &ResultE, 1);
+ Result = InitSeq.Perform(*this, Entity, Kind,
+ MultiExprArg(*this, &ResultE, 1));
+ if (Result.isInvalid())
+ return ExprError();
- // If the default expression creates temporaries, we need to
- // push them to the current stack of expression temporaries so they'll
- // be properly destroyed.
- // FIXME: We should really be rebuilding the default argument with new
- // bound temporaries; see the comment in PR5810.
- for (unsigned i = 0, e = Param->getNumDefaultArgTemporaries(); i != e; ++i)
- ExprTemporaries.push_back(Param->getDefaultArgTemporary(i));
+ // Build the default argument expression.
+ return Owned(CXXDefaultArgExpr::Create(Context, CallLoc, Param,
+ Result.takeAs<Expr>()));
}
- // We already type-checked the argument, so we know it works.
+ // If the default expression creates temporaries, we need to
+ // push them to the current stack of expression temporaries so they'll
+ // be properly destroyed.
+ // FIXME: We should really be rebuilding the default argument with new
+ // bound temporaries; see the comment in PR5810.
+ for (unsigned i = 0, e = Param->getNumDefaultArgTemporaries(); i != e; ++i)
+ ExprTemporaries.push_back(Param->getDefaultArgTemporary(i));
+
+ // We already type-checked the argument, so we know it works.
+ MarkDeclarationsReferencedInExpr(Param->getDefaultArg());
return Owned(CXXDefaultArgExpr::Create(Context, CallLoc, Param));
}
@@ -7653,6 +7656,11 @@
// potentially evaluated.
ExprEvalContexts.back().addReferencedDecl(Loc, D);
return;
+
+ case PotentiallyEvaluatedIfUsed:
+ // Referenced declarations will only be used if the construct in the
+ // containing expression is used.
+ return;
}
// Note that this declaration has been used.
@@ -7791,6 +7799,55 @@
Marker.TraverseType(Context.getCanonicalType(T));
}
+namespace {
+ /// \brief Helper class that marks all of the declarations referenced by
+ /// potentially-evaluated subexpressions as "referenced".
+ class EvaluatedExprMarker : public EvaluatedExprVisitor<EvaluatedExprMarker> {
+ Sema &S;
+
+ public:
+ typedef EvaluatedExprVisitor<EvaluatedExprMarker> Inherited;
+
+ explicit EvaluatedExprMarker(Sema &S) : Inherited(S.Context), S(S) { }
+
+ void VisitDeclRefExpr(DeclRefExpr *E) {
+ S.MarkDeclarationReferenced(E->getLocation(), E->getDecl());
+ }
+
+ void VisitMemberExpr(MemberExpr *E) {
+ S.MarkDeclarationReferenced(E->getMemberLoc(), E->getMemberDecl());
+ }
+
+ void VisitCXXNewExpr(CXXNewExpr *E) {
+ if (E->getConstructor())
+ S.MarkDeclarationReferenced(E->getLocStart(), E->getConstructor());
+ if (E->getOperatorNew())
+ S.MarkDeclarationReferenced(E->getLocStart(), E->getOperatorNew());
+ if (E->getOperatorDelete())
+ S.MarkDeclarationReferenced(E->getLocStart(), E->getOperatorDelete());
+ }
+
+ void VisitCXXDeleteExpr(CXXDeleteExpr *E) {
+ if (E->getOperatorDelete())
+ S.MarkDeclarationReferenced(E->getLocStart(), E->getOperatorDelete());
+ }
+
+ void VisitCXXConstructExpr(CXXConstructExpr *E) {
+ S.MarkDeclarationReferenced(E->getLocStart(), E->getConstructor());
+ }
+
+ void VisitBlockDeclRefExpr(BlockDeclRefExpr *E) {
+ S.MarkDeclarationReferenced(E->getLocation(), E->getDecl());
+ }
+ };
+}
+
+/// \brief Mark any declarations that appear within this expression or any
+/// potentially-evaluated subexpressions as "referenced".
+void Sema::MarkDeclarationsReferencedInExpr(Expr *E) {
+ EvaluatedExprMarker(*this).Visit(E);
+}
+
/// \brief Emit a diagnostic that describes an effect on the run-time behavior
/// of the program being compiled.
///
@@ -7815,6 +7872,7 @@
break;
case PotentiallyEvaluated:
+ case PotentiallyEvaluatedIfUsed:
Diag(Loc, PD);
return true;
Modified: cfe/trunk/test/SemaTemplate/default-expr-arguments.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/default-expr-arguments.cpp?rev=113700&r1=113699&r2=113700&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/default-expr-arguments.cpp (original)
+++ cfe/trunk/test/SemaTemplate/default-expr-arguments.cpp Sat Sep 11 15:24:53 2010
@@ -206,3 +206,31 @@
Holder<int> h(i);
}
};
+
+namespace PR5810b {
+ template<typename T>
+ T broken() {
+ T t;
+ double**** not_it = t;
+ }
+
+ void f(int = broken<int>());
+ void g() { f(17); }
+}
+
+namespace PR8127 {
+ template< typename T > class PointerClass {
+ public:
+ PointerClass( T * object_p ) : p_( object_p ) {
+ p_->acquire();
+ }
+ private:
+ T * p_;
+ };
+
+ class ExternallyImplementedClass;
+
+ class MyClass {
+ void foo( PointerClass<ExternallyImplementedClass> = 0 );
+ };
+}
More information about the cfe-commits
mailing list