[clang] Fix ICE with typeid & polymorphic class (pr50497)

[urnathan]

Summary

This addresses pr50497. The argument of a typeid expression is unevaluated, *except* when it's a polymorphic type. We handle this by parsing as unevaluated and then transforming to evaluated if we discover it should have been an evaluated context.

We do the same in TreeTransform<Derived>::TransformCXXTypeidExpr, entering unevaluated context before transforming and rebuilding the typeid. But that's incorrect and can lead us to converting to evaluated context twice -- and hitting an assert.

During normal template instantiation we're always cloning the expression, but during generic lambda processing we do not necessarily AlwaysRebuild, and end up with TransformDeclRefExpr unconditionally calling MarkDeclRefReferenced around line 10226. That triggers the assert.

// Mark it referenced in the new context regardless.
// FIXME: this is a bit instantiation-specific.
SemaRef.MarkDeclRefReferenced(E);

This patch makes 2 changes.
a) TreeTransform<Derived>::TransformCXXTypeidExpr only enters unevaluated context if the typeid's operand is not a polymorphic glvalue. If it is, it keeps the same evaluation context.
b) Sema::BuildCXXTypeId is altered to only transform to evaluated, if the current context is unevaluated.

[bruno]

LGTM

clang/lib/Sema/SemaExprCXX.cpp

if (const RecordType *RecordT = T->getAs<RecordType>()) {
       if (RequireCompleteType(TypeidLoc, T, diag::err_incomplete_typeid))
         return ExprError();
 
       // C++ [expr.typeid]p3:
       //   When typeid is applied to an expression other than an glvalue of a
       //   polymorphic class type [...] [the] expression is an unevaluated
       //   operand. [...]
       if (RecordD->isPolymorphic() && E->isGLValue()) {
-        // The subexpression is potentially evaluated; switch the context
-        // and recheck the subexpression.
+        if (isUnevaluatedContext()) {
+          // The operand was processed in unevaluated context, switch the
+          // context and recheck the subexpression.
           ExprResult Result = TransformToPotentiallyEvaluated(E);
-        if (Result.isInvalid()) return ExprError();
+          if (Result.isInvalid())
+            return ExprError();
           E = Result.get();
+        }
 
         // We require a vtable to query the type at run time.
         MarkVTableUsed(TypeidLoc, RecordD);
         WasEvaluated = true;
       }
     }
 
     ExprResult Result = CheckUnevaluatedOperand(E);

clang/lib/Sema/TreeTransform.h

if (E->isTypeOperand()) {
     if (!getDerived().AlwaysRebuild() &&
         TInfo == E->getTypeOperandSourceInfo())
       return E;
 
     return getDerived().RebuildCXXTypeidExpr(E->getType(), E->getBeginLoc(),
                                              TInfo, E->getEndLoc());
   }
 
-  // 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::ExpressionEvaluationContext::Unevaluated,
+  // Typeid's operand is an unevaluated context, unless it's a polymorphic
+  // type.  We must not unilaterally enter unevaluated context here, as then
+  // semantic processing can re-transform an already transformed operand.
+  Expr *Op = E->getExprOperand();
+  auto EvalCtx = Sema::ExpressionEvaluationContext::Unevaluated;
+  if (E->isGLValue())
+    if (auto *RecordT = Op->getType()->getAs<RecordType>())
+      if (cast<CXXRecordDecl>(RecordT->getDecl())->isPolymorphic())
+        EvalCtx = SemaRef.ExprEvalContexts.back().Context;
+
+  EnterExpressionEvaluationContext Unevaluated(SemaRef, EvalCtx,
                                                Sema::ReuseLambdaContextDecl);
 
-  ExprResult SubExpr = getDerived().TransformExpr(E->getExprOperand());
+  ExprResult SubExpr = getDerived().TransformExpr(Op);
   if (SubExpr.isInvalid())
     return ExprError();
 
   if (!getDerived().AlwaysRebuild() &&
       SubExpr.get() == E->getExprOperand())
     return E;
 
   return getDerived().RebuildCXXTypeidExpr(E->getType(), E->getBeginLoc(),

clang/test/SemaCXX/pr50497-crash-typeid.cpp

+// RUN: %clang_cc1 -verify %s -Wno-unevaluated-expression
+// Don't crash (PR50497).
+
+// expected-no-diagnostics
+namespace std {
+class type_info;
+}
+
+class Ex {
+  // polymorphic
+  virtual ~Ex();
+};
+void Frob(const std::type_info &type);
+
+void Foo(Ex *ex) {
+  // generic lambda
+  [=](auto rate) {
+    // typeid
+    Frob(typeid(*ex));
+  }(1);
+
+  [=](auto rate) {
+    // unevaluated nested typeid
+    Frob(typeid((typeid(*ex), ex)));
+  }(1);
+}