[cfe-commits] [PATCH] Remove PotentiallyPotentiallyEvaluated
Eli Friedman
eli.friedman at gmail.com
Wed Jan 18 16:53:47 PST 2012
Per subject, attached patch removes PotentiallyPotentiallyEvaluated
and use TreeTransform to transform expressions from unevaluated to
potentially evaluated contexts. The primary advantages are that this
allows removing a bunch of code, and fixes our handling of some edge
cases (specifically, cases involving lvalue-to-rvalue conversion of
classes, and a few edge cases of ARC lifetime deduction in
potentially-potentially-evaluated contexts with template
instantiation). The disadvantage are performance and codesize: we end
up redoing semantic checking in some cases where there isn't any
semantic checking necessary, and TreeTransform itself is a non-trivial
template. If performance is an issue, I could add code to avoid
actually performing the TreeTransform in some cases where we know it
will generate an identical AST. That said, this is unlikely to have a
measurable performance impact anyway: we should only hit this codepath
for typeid of an expression of polymorphic type, sizeof on VLA types,
and typeof on variably modified types (which are all rare, and the
subexpressions should generally be small).
Is there anything I'm missing in my analysis? If I'm not missing
anything, are the disadvantages large enough that we want another
approach?
This patch passes all the regression tests except for one,
CXX/expr/expr.prim/expr.prim.general/p12-0x.cpp, which I haven't
figured out how to fix. (The issue is that given a DeclRefExpr
pointing at a FieldDecl, there isn't any easy way to distinguish
whether it is part of member pointer formation.)
-Eli
-------------- next part --------------
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 148419)
+++ include/clang/Sema/Sema.h (working copy)
@@ -511,21 +511,6 @@
/// call was found yet.
bool ObjCShouldCallSuperFinalize;
- /// \brief The set of declarations that have been referenced within
- /// a potentially evaluated expression.
- typedef SmallVector<std::pair<SourceLocation, Decl *>, 10>
- PotentiallyReferencedDecls;
-
- /// \brief A set of diagnostics that may be emitted.
- typedef SmallVector<std::pair<SourceLocation, PartialDiagnostic>, 10>
- PotentiallyEmittedDiagnostics;
-
- typedef SmallVector<sema::DelayedDiagnostic, 10>
- PotentiallyEmittedDelayedDiag;
-
- typedef SmallVector<sema::PossiblyUnreachableDiag, 10>
- PotentiallyEmittedPossiblyUnreachableDiag;
-
/// \brief Describes how the expressions currently being parsed are
/// evaluated at run-time, if at all.
enum ExpressionEvaluationContext {
@@ -546,13 +531,6 @@
/// expression at run time.
PotentiallyEvaluated,
- /// \brief The current expression may be potentially evaluated or it may
- /// be unevaluated, but it is impossible to tell from the lexical context.
- /// 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,
-
/// \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.
@@ -577,43 +555,11 @@
/// this expression evaluation context.
unsigned NumCleanupObjects;
- /// \brief The set of declarations referenced within a
- /// potentially potentially-evaluated context.
- ///
- /// When leaving a potentially potentially-evaluated context, each
- /// of these elements will be as referenced if the corresponding
- /// potentially potentially evaluated expression is potentially
- /// evaluated.
- PotentiallyReferencedDecls *PotentiallyReferenced;
-
- // There are three kinds of diagnostics we care about in
- // PotentiallyPotentiallyEvaluated contexts: regular Diag diagnostics,
- // DelayedDiagnostics, and DiagRuntimeBehavior diagnostics.
- PotentiallyEmittedDiagnostics *SavedDiag;
- PotentiallyEmittedDelayedDiag *SavedDelayedDiag;
- PotentiallyEmittedPossiblyUnreachableDiag *SavedRuntimeDiag;
-
ExpressionEvaluationContextRecord(ExpressionEvaluationContext Context,
unsigned NumCleanupObjects,
bool ParentNeedsCleanups)
: Context(Context), ParentNeedsCleanups(ParentNeedsCleanups),
- NumCleanupObjects(NumCleanupObjects),
- PotentiallyReferenced(0), SavedDiag(0), SavedDelayedDiag(0),
- SavedRuntimeDiag(0) { }
-
- void addReferencedDecl(SourceLocation Loc, Decl *Decl) {
- if (!PotentiallyReferenced)
- PotentiallyReferenced = new PotentiallyReferencedDecls;
- PotentiallyReferenced->push_back(std::make_pair(Loc, Decl));
- }
-
- void addDiagnostic(SourceLocation Loc, const PartialDiagnostic &PD);
-
- void addRuntimeDiagnostic(const sema::PossiblyUnreachableDiag &PUD);
-
- void addDelayedDiagnostic(const sema::DelayedDiagnostic &DD);
-
- void Destroy();
+ NumCleanupObjects(NumCleanupObjects) { }
};
/// A stack of expression evaluation contexts.
@@ -2335,6 +2281,8 @@
void DiscardCleanupsInEvaluationContext();
+ ExprResult TranformToPotentiallyEvaluated(Expr *E);
+
void MarkDeclarationReferenced(SourceLocation Loc, Decl *D);
void MarkDeclarationsReferencedInType(SourceLocation Loc, QualType T);
void MarkDeclarationsReferencedInExpr(Expr *E);
@@ -3142,8 +3090,7 @@
QualType getCurrentThisType();
/// \brief Make sure the value of 'this' is actually available in the current
- /// context, if it is a potentially evaluated context. This check can be
- /// delayed in PotentiallyPotentiallyEvaluated contexts.
+ /// context, if it is a potentially evaluated context.
void CheckCXXThisCapture(SourceLocation Loc);
/// ActOnCXXBoolLiteral - Parse {true,false} literals.
Index: lib/Sema/TreeTransform.h
===================================================================
--- lib/Sema/TreeTransform.h (revision 148419)
+++ lib/Sema/TreeTransform.h (working copy)
@@ -6881,8 +6881,7 @@
// We don't know whether the expression is potentially evaluated until
// after we perform semantic analysis, so the expression is potentially
// potentially evaluated.
- EnterExpressionEvaluationContext Unevaluated(SemaRef,
- Sema::PotentiallyPotentiallyEvaluated);
+ EnterExpressionEvaluationContext Unevaluated(SemaRef, Sema::Unevaluated);
ExprResult SubExpr = getDerived().TransformExpr(E->getExprOperand());
if (SubExpr.isInvalid())
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp (revision 148419)
+++ lib/Sema/SemaExprCXX.cpp (working copy)
@@ -310,7 +310,6 @@
SourceLocation TypeidLoc,
Expr *E,
SourceLocation RParenLoc) {
- bool isUnevaluatedOperand = true;
if (E && !E->isTypeDependent()) {
if (E->getType()->isPlaceholderType()) {
ExprResult result = CheckPlaceholderExpr(E);
@@ -332,7 +331,11 @@
// polymorphic class type [...] [the] expression is an unevaluated
// operand. [...]
if (RecordD->isPolymorphic() && E->Classify(Context).isGLValue()) {
- isUnevaluatedOperand = false;
+ // The subexpression is potentially evaluated; switch the context
+ // and recheck the subexpression.
+ ExprResult Result = TranformToPotentiallyEvaluated(E);
+ if (Result.isInvalid()) return ExprError();
+ E = Result.take();
// We require a vtable to query the type at run time.
MarkVTableUsed(TypeidLoc, RecordD);
@@ -352,12 +355,6 @@
}
}
- // If this is an unevaluated operand, clear out the set of
- // declaration references we have been computing and eliminate any
- // temporaries introduced in its computation.
- if (isUnevaluatedOperand)
- ExprEvalContexts.back().Context = Unevaluated;
-
return Owned(new (Context) CXXTypeidExpr(TypeInfoType.withConst(),
E,
SourceRange(TypeidLoc, RParenLoc)));
@@ -695,14 +692,7 @@
continue;
}
// This context can't implicitly capture 'this'; fail out.
- // (We need to delay the diagnostic in the
- // PotentiallyPotentiallyEvaluated case because it doesn't apply to
- // unevaluated contexts.)
- if (ExprEvalContexts.back().Context == PotentiallyPotentiallyEvaluated)
- ExprEvalContexts.back()
- .addDiagnostic(Loc, PDiag(diag::err_implicit_this_capture));
- else
- Diag(Loc, diag::err_implicit_this_capture);
+ Diag(Loc, diag::err_implicit_this_capture);
return;
}
break;
Index: lib/Sema/SemaType.cpp
===================================================================
--- lib/Sema/SemaType.cpp (revision 148419)
+++ lib/Sema/SemaType.cpp (working copy)
@@ -1057,40 +1057,23 @@
} else if (type->isObjCARCImplicitlyUnretainedType()) {
implicitLifetime = Qualifiers::OCL_ExplicitNone;
- // If we are in an unevaluated context, like sizeof, assume Autoreleasing and
- // don't give error.
+ // If we are in an unevaluated context, like sizeof, skip adding a
+ // qualification.
} else if (S.ExprEvalContexts.back().Context == Sema::Unevaluated ||
S.ExprEvalContexts.back().Context == Sema::ConstantEvaluated) {
- implicitLifetime = Qualifiers::OCL_Autoreleasing;
+ return type;
// If that failed, give an error and recover using __autoreleasing.
} else {
// These types can show up in private ivars in system headers, so
// we need this to not be an error in those cases. Instead we
// want to delay.
- //
- // Also, make sure we delay appropriately in
- // PotentiallyPotentiallyEvaluated contexts.
if (S.DelayedDiagnostics.shouldDelayDiagnostics()) {
- if (S.ExprEvalContexts.back().Context ==
- Sema::PotentiallyPotentiallyEvaluated) {
- S.ExprEvalContexts.back().addDelayedDiagnostic(
- sema::DelayedDiagnostic::makeForbiddenType(loc,
- diag::err_arc_indirect_no_ownership, type, isReference));
- } else {
- S.DelayedDiagnostics.add(
- sema::DelayedDiagnostic::makeForbiddenType(loc,
- diag::err_arc_indirect_no_ownership, type, isReference));
- }
+ S.DelayedDiagnostics.add(
+ sema::DelayedDiagnostic::makeForbiddenType(loc,
+ diag::err_arc_indirect_no_ownership, type, isReference));
} else {
- if (S.ExprEvalContexts.back().Context ==
- Sema::PotentiallyPotentiallyEvaluated) {
- S.ExprEvalContexts.back().addDiagnostic(loc,
- S.PDiag(diag::err_arc_indirect_no_ownership)
- << type << isReference);
- } else {
- S.Diag(loc, diag::err_arc_indirect_no_ownership) << type << isReference;
- }
+ S.Diag(loc, diag::err_arc_indirect_no_ownership) << type << isReference;
}
implicitLifetime = Qualifiers::OCL_Autoreleasing;
}
Index: lib/Sema/SemaExprMember.cpp
===================================================================
--- lib/Sema/SemaExprMember.cpp (revision 148419)
+++ lib/Sema/SemaExprMember.cpp (working copy)
@@ -74,15 +74,10 @@
/// context is not an instance method.
IMA_Unresolved_StaticContext,
- // The reference is an instance data member access, which is allowed
- // because we're in C++11 mode and the context is unevaluated.
+ // The reference is to a field which is not an instance data member, which
+ // is allowed because we're in C++11 mode and the context is unevaluated.
IMA_Field_Uneval_StaticContext,
- // The reference is an instance data member access, which may be allowed
- // because we're in C++11 mode and the context may be unevaluated
- // (i.e. the context is PotentiallyPotentiallyEvaluated).
- IMA_Field_PPE_StaticContext,
-
/// All possible referrents are instance members and the current
/// context is not an instance method.
IMA_Error_StaticContext,
@@ -159,8 +154,6 @@
= SemaRef.ExprEvalContexts.back();
if (record.Context == Sema::Unevaluated)
return IMA_Field_Uneval_StaticContext;
- if (record.Context == Sema::PotentiallyPotentiallyEvaluated)
- return IMA_Field_PPE_StaticContext;
}
return IMA_Error_StaticContext;
@@ -196,8 +189,7 @@
static void DiagnoseInstanceReference(Sema &SemaRef,
const CXXScopeSpec &SS,
NamedDecl *rep,
- const DeclarationNameInfo &nameInfo,
- bool DelayPPEDiag = false) {
+ const DeclarationNameInfo &nameInfo) {
SourceLocation Loc = nameInfo.getLoc();
SourceRange Range(Loc);
if (SS.isSet()) Range.setBegin(SS.getRange().getBegin());
@@ -206,29 +198,17 @@
if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(SemaRef.CurContext)) {
if (MD->isStatic()) {
// "invalid use of member 'x' in static member function"
- if (DelayPPEDiag)
- SemaRef.ExprEvalContexts.back().addDiagnostic(Loc,
- SemaRef.PDiag(diag::err_invalid_member_use_in_static_method)
- << Range << nameInfo.getName());
- else
- SemaRef.Diag(Loc, diag::err_invalid_member_use_in_static_method)
- << Range << nameInfo.getName();
+ SemaRef.Diag(Loc, diag::err_invalid_member_use_in_static_method)
+ << Range << nameInfo.getName();
return;
}
}
- if (DelayPPEDiag)
- SemaRef.ExprEvalContexts.back().addDiagnostic(Loc,
- SemaRef.PDiag(diag::err_invalid_non_static_member_use)
- << nameInfo.getName() << Range);
- else
- SemaRef.Diag(Loc, diag::err_invalid_non_static_member_use)
- << nameInfo.getName() << Range;
+ SemaRef.Diag(Loc, diag::err_invalid_non_static_member_use)
+ << nameInfo.getName() << Range;
return;
}
- assert(!DelayPPEDiag && "Only need to delay diagnostic for fields");
-
SemaRef.Diag(Loc, diag::err_member_call_without_object) << Range;
}
@@ -246,11 +226,6 @@
case IMA_Unresolved:
return BuildImplicitMemberExpr(SS, R, TemplateArgs, false);
- case IMA_Field_PPE_StaticContext:
- DiagnoseInstanceReference(*this, SS, R.getRepresentativeDecl(),
- R.getLookupNameInfo(), /*DelayPPEDiag*/true);
- // FALL-THROUGH
-
case IMA_Static:
case IMA_Mixed_StaticContext:
case IMA_Unresolved_StaticContext:
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp (revision 148419)
+++ lib/Sema/SemaExpr.cpp (working copy)
@@ -40,6 +40,7 @@
#include "clang/Sema/ParsedTemplate.h"
#include "clang/Sema/SemaFixItUtils.h"
#include "clang/Sema/Template.h"
+#include "TreeTransform.h"
using namespace clang;
using namespace sema;
@@ -1208,10 +1209,6 @@
case Sema::PotentiallyEvaluated:
case Sema::PotentiallyEvaluatedIfUsed:
break;
-
- case Sema::PotentiallyPotentiallyEvaluated:
- // FIXME: delay these!
- break;
}
// Don't diagnose about capture if we're not actually in code right
@@ -9361,39 +9358,28 @@
return false;
}
-void Sema::ExpressionEvaluationContextRecord::Destroy() {
- delete PotentiallyReferenced;
- delete SavedDiag;
- delete SavedRuntimeDiag;
- delete SavedDelayedDiag;
- PotentiallyReferenced = 0;
- SavedDiag = 0;
- SavedRuntimeDiag = 0;
- SavedDelayedDiag = 0;
-}
+namespace {
+ // Handle the case where we conclude a potentially-potentially-evaluated
+ // expression is actually evaluated.
+ class PPEToPETransform :
+ public TreeTransform<PPEToPETransform> {
+ public:
+ PPEToPETransform(Sema &SemaRef) :
+ TreeTransform<PPEToPETransform>(SemaRef) {
+ }
-void Sema::ExpressionEvaluationContextRecord::addDiagnostic(
- SourceLocation Loc, const PartialDiagnostic &PD) {
- if (!SavedDiag)
- SavedDiag = new PotentiallyEmittedDiagnostics;
- SavedDiag->push_back(std::make_pair(Loc, PD));
+ bool AlwaysRebuild() { return true; }
+ };
}
-void Sema::ExpressionEvaluationContextRecord::addRuntimeDiagnostic(
- const sema::PossiblyUnreachableDiag &PUD) {
- if (!SavedRuntimeDiag)
- SavedRuntimeDiag = new PotentiallyEmittedPossiblyUnreachableDiag;
- SavedRuntimeDiag->push_back(PUD);
+ExprResult Sema::TranformToPotentiallyEvaluated(Expr *E) {
+ ExprEvalContexts.back().Context =
+ ExprEvalContexts[ExprEvalContexts.size()-2].Context;
+ if (ExprEvalContexts.back().Context == Unevaluated)
+ return E;
+ return PPEToPETransform(*this).TransformExpr(E).take();
}
-void Sema::ExpressionEvaluationContextRecord::addDelayedDiagnostic(
- const sema::DelayedDiagnostic &DD) {
- if (!SavedDelayedDiag)
- SavedDelayedDiag = new PotentiallyEmittedDelayedDiag;
- SavedDelayedDiag->push_back(DD);
-}
-
-
void
Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext) {
ExprEvalContexts.push_back(
@@ -9408,46 +9394,6 @@
ExpressionEvaluationContextRecord Rec = ExprEvalContexts.back();
ExprEvalContexts.pop_back();
- if (Rec.Context == PotentiallyPotentiallyEvaluated) {
- if (Rec.PotentiallyReferenced) {
- // Mark any remaining declarations in the current position of the stack
- // as "referenced". If they were not meant to be referenced, semantic
- // analysis would have eliminated them (e.g., in ActOnCXXTypeId).
- for (PotentiallyReferencedDecls::iterator
- I = Rec.PotentiallyReferenced->begin(),
- IEnd = Rec.PotentiallyReferenced->end();
- I != IEnd; ++I)
- MarkDeclarationReferenced(I->first, I->second);
- }
-
- if (Rec.SavedDiag) {
- // Emit any pending diagnostics.
- for (PotentiallyEmittedDiagnostics::iterator
- I = Rec.SavedDiag->begin(),
- IEnd = Rec.SavedDiag->end();
- I != IEnd; ++I)
- Diag(I->first, I->second);
- }
-
- if (Rec.SavedDelayedDiag) {
- // Emit any pending delayed diagnostics.
- for (PotentiallyEmittedDelayedDiag::iterator
- I = Rec.SavedDelayedDiag->begin(),
- IEnd = Rec.SavedDelayedDiag->end();
- I != IEnd; ++I)
- DelayedDiagnostics.add(*I);
- }
-
- if (Rec.SavedRuntimeDiag) {
- // Emit any pending runtime diagnostics.
- for (PotentiallyEmittedPossiblyUnreachableDiag::iterator
- I = Rec.SavedRuntimeDiag->begin(),
- IEnd = Rec.SavedRuntimeDiag->end();
- I != IEnd; ++I)
- FunctionScopes.back()->PossiblyUnreachableDiags.push_back(*I);
- }
- }
-
// When are coming out of an unevaluated context, clear out any
// temporaries that we may have created as part of the evaluation of
// the expression in that context: they aren't relevant because they
@@ -9461,9 +9407,6 @@
} else {
ExprNeedsCleanups |= Rec.ParentNeedsCleanups;
}
-
- // Destroy the popped expression evaluation record.
- Rec.Destroy();
}
void Sema::DiscardCleanupsInEvaluationContext() {
@@ -9519,13 +9462,6 @@
// "used"; handle this below.
break;
- case PotentiallyPotentiallyEvaluated:
- // We are in an expression that may be potentially evaluated; queue this
- // declaration reference until we know whether the expression is
- // 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.
@@ -9809,11 +9745,6 @@
Diag(Loc, PD);
return true;
-
- case PotentiallyPotentiallyEvaluated:
- ExprEvalContexts.back().addRuntimeDiagnostic(
- sema::PossiblyUnreachableDiag(PD, Loc, Statement));
- break;
}
return false;
Index: lib/Parse/ParseExprCXX.cpp
===================================================================
--- lib/Parse/ParseExprCXX.cpp (revision 148419)
+++ lib/Parse/ParseExprCXX.cpp (working copy)
@@ -926,8 +926,7 @@
// Note that we can't tell whether the expression is an lvalue of a
// polymorphic class type until after we've parsed the expression, so
// we the expression is potentially potentially evaluated.
- EnterExpressionEvaluationContext Unevaluated(Actions,
- Sema::PotentiallyPotentiallyEvaluated);
+ EnterExpressionEvaluationContext Unevaluated(Actions, Sema::Unevaluated);
Result = ParseExpression();
// Match the ')'.
More information about the cfe-commits
mailing list