[cfe-commits] [PATCH] Remove PotentiallyPotentiallyEvaluated

Eli Friedman eli.friedman at gmail.com
Thu Jan 19 10:57:59 PST 2012


On Wed, Jan 18, 2012 at 9:50 PM, John McCall <rjmccall at apple.com> wrote:
> Also, would you mind measuring the code-size increase from adding a new
> instance of TreeTransform?

Roughly 74KB in a Release build.  If we care, TreeTransform could be
optimized to reduce the penalty.

> +    // Error on DeclRefExprs referring to FieldDecls.
> +    // FIXME: This actually isn't supposed to be an error!
> +    ExprResult TransformDeclRefExpr(DeclRefExpr *E) {
>
> What's up with this FIXME?

My mistake; fixing the remaining field-reference-in-unevaluated cases
won't require any changes here.

Fixed version attached.

-Eli
-------------- next part --------------
Index: lib/Sema/TreeTransform.h
===================================================================
--- lib/Sema/TreeTransform.h	(revision 148419)
+++ lib/Sema/TreeTransform.h	(working copy)
@@ -6878,11 +6878,11 @@
                                              E->getLocEnd());
   }
 
-  // We don't know whether the expression is potentially evaluated until
-  // after we perform semantic analysis, so the expression is potentially
+  // We don't know whether the subexpression is potentially evaluated until
+  // after we perform semantic analysis.  We speculatively assume it is
+  // unevaluated; it will get fixed later if the subexpression is in fact
   // 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,11 @@
   /// 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.
-  IMA_Field_Uneval_StaticContext,
+  // The reference refers to a field which is not a member of the containing
+  // class, which is allowed because we're in C++11 mode and the context is
+  // unevaluated.
+  IMA_Field_Uneval_Context,
 
-  // 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,
@@ -158,9 +154,7 @@
       const Sema::ExpressionEvaluationContextRecord& record
         = 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_Field_Uneval_Context;
     }
     
     return IMA_Error_StaticContext;
@@ -196,8 +190,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 +199,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,15 +227,10 @@
   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:
-  case IMA_Field_Uneval_StaticContext:
+  case IMA_Field_Uneval_Context:
     if (TemplateArgs)
       return BuildTemplateIdExpr(SS, R, false, *TemplateArgs);
     return BuildDeclarationNameExpr(SS, R, false);
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,52 @@
   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 expression which we speculatively
+  // considered to be unevaluated is actually evaluated.
+  class TransformToPE : public TreeTransform<TransformToPE> {
+    typedef TreeTransform<TransformToPE> BaseTransform;
 
-void Sema::ExpressionEvaluationContextRecord::addDiagnostic(
-        SourceLocation Loc, const PartialDiagnostic &PD) {
-  if (!SavedDiag)
-    SavedDiag = new PotentiallyEmittedDiagnostics;
-  SavedDiag->push_back(std::make_pair(Loc, PD));
-}
+  public:
+    TransformToPE(Sema &SemaRef) : BaseTransform(SemaRef) { }
 
-void Sema::ExpressionEvaluationContextRecord::addRuntimeDiagnostic(
-        const sema::PossiblyUnreachableDiag &PUD) {
-  if (!SavedRuntimeDiag)
-    SavedRuntimeDiag = new PotentiallyEmittedPossiblyUnreachableDiag;
-  SavedRuntimeDiag->push_back(PUD);
+    // Make sure we redo semantic analysis
+    bool AlwaysRebuild() { return true; }
+
+    // We need to special-case DeclRefExprs referring to FieldDecls which
+    // are not part of a member pointer formation; normal TreeTransforming
+    // doesn't catch this case because of the way we represent them in the AST.
+    // FIXME: This is a bit ugly; is it really the best way to handle this
+    // case?
+    // 
+    // Filter out member pointer formation
+    ExprResult TransformUnaryOperator(UnaryOperator *E) {
+      if (E->getOpcode() == UO_AddrOf && E->getType()->isMemberPointerType())
+        return E;
+
+      return BaseTransform::TransformUnaryOperator(E);
+    }
+
+    // Error on DeclRefExprs referring to FieldDecls.
+    ExprResult TransformDeclRefExpr(DeclRefExpr *E) {
+      if (isa<FieldDecl>(E->getDecl()))
+        return SemaRef.Diag(E->getLocation(),
+                            diag::err_invalid_non_static_member_use)
+            << E->getDecl() << E->getSourceRange();
+
+      return BaseTransform::TransformDeclRefExpr(E);
+    }
+  };
 }
 
-void Sema::ExpressionEvaluationContextRecord::addDelayedDiagnostic(
-        const sema::DelayedDiagnostic &DD) {
-  if (!SavedDelayedDiag)
-    SavedDelayedDiag = new PotentiallyEmittedDelayedDiag;
-  SavedDelayedDiag->push_back(DD);
+ExprResult Sema::TranformToPotentiallyEvaluated(Expr *E) {
+  ExprEvalContexts.back().Context =
+      ExprEvalContexts[ExprEvalContexts.size()-2].Context;
+  if (ExprEvalContexts.back().Context == Unevaluated)
+    return E;
+  return TransformToPE(*this).TransformExpr(E);
 }
 
-
 void
 Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext) {
   ExprEvalContexts.push_back(
@@ -9408,46 +9418,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 +9431,6 @@
   } else {
     ExprNeedsCleanups |= Rec.ParentNeedsCleanups;
   }
-
-  // Destroy the popped expression evaluation record.
-  Rec.Destroy();
 }
 
 void Sema::DiscardCleanupsInEvaluationContext() {
@@ -9519,13 +9486,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 +9769,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)
@@ -924,10 +924,10 @@
     //   operand (Clause 5).
     //
     // 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);
+    // polymorphic class type until after we've parsed the expression; we
+    // speculatively assume the subexpression is unevaluated, and fix it up
+    // later.
+    EnterExpressionEvaluationContext Unevaluated(Actions, Sema::Unevaluated);
     Result = ParseExpression();
 
     // Match the ')'.
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.


More information about the cfe-commits mailing list