[CodeGen][typeid] Emit typeinfo directly if type is known at compile-time

[zequanwu]

Summary

Bug: https://bugs.llvm.org/show_bug.cgi?id=47467

[zequanwu]

Remove unused field.

[hans]

I'm not sure that changing isPotentiallyEvaluated() is the right thing to do. The meaning of that corresponds to text in the standard: https://eel.is/c++draft/expr.typeid#3 so changing it to something that doesn't match the standard exactly seems wrong.

I think it would be safer to do the change purely as an optimization in codegen (maybe we could add a new helper method that could also be used by the warning).

Also, I found another interesting test case: https://godbolt.org/z/aW4ojr

#include <typeinfo>

class B {
public:
  virtual void f();
};

B f();

 const std::type_info& g() {
  return typeid(f());
}

Here MSVC warns about /GR- and emits the call to __RTtypeid even though I think it's not necessary since the type is statically known. Actually, if I understand things correctly, f() shouldn't even be evaluated here since the expression is not a glvalue, and Clang gets that right. So that's a case where our codegen is better than MSVC :-)

[zequanwu]

If the operand of CXXTypeidExpr is already most derived object, no need to look up vtable.

[zequanwu]

In D87425#2265523, @hans wrote:

I'm not sure that changing isPotentiallyEvaluated() is the right thing to do. The meaning of that corresponds to text in the standard: https://eel.is/c++draft/expr.typeid#3 so changing it to something that doesn't match the standard exactly seems wrong.

If the type of the operand is already most derived, we could just emit the typeinfo of the type. This would match the standard: https://eel.is/c++draft/expr.typeid#2

I think it would be safer to do the change purely as an optimization in codegen (maybe we could add a new helper method that could also be used by the warning).

For "optimization in codegen", do you mean optimization after the IR is generated or like I did in CodeGenFunction::EmitCXXTypeidExpr?

[hans]

I think it would be safer to do the change purely as an optimization in codegen (maybe we could add a new helper method that could also be used by the warning).

For "optimization in codegen", do you mean optimization after the IR is generated or like I did in CodeGenFunction::EmitCXXTypeidExpr?

I mean like you did in CodeGenFunction::EmitCXXTypeidExpr.

clang/include/clang/AST/ExprCXX.h

844

A comment would be good here.

clang/lib/AST/ExprCXX.cpp

157

I suppose it depends on what you want the semantics of this function to be, but returning false here seems odd.. wouldn't true make as much sense?
If this function is not intended to be called for type operands, maybe assert about that instead?

160

I think checking for DeclRefExpr isn't guaranteed to handle all cases, like

typeid(*&obj)

for example. That's okay, but it should be clear from the function comment (and maybe the name) that this is a best-effort check, not a strong guarantee about whether the expression refers to a most derived object.

clang/test/SemaCXX/constant-expression-cxx2a.cpp

312

This appears to be changing semantics. It could be that this test is unnecessary strict (that's my understanding), but usually these checks are based on examples in the standard, or the current understanding of the standard. I think it would be best to check with Richard before changing this.

Actually, I'm surprised this affected since you're only updating CodeGen, not Sema. Is the static_assert really invoking Codegen?

[zequanwu]

Address comment.

[zequanwu]

clang/test/SemaCXX/constant-expression-cxx2a.cpp

312

This was caused by my previous changes on isPotentiallyEvaluated, which is used by Sema. I reverted this.

[hans]

lgtm

clang/test/SemaCXX/constant-expression-cxx2a.cpp

312

Ah, great. That makes more sense :)

clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp

if (const auto *UnaryExpr = dyn_cast<UnaryExprOrTypeTraitExpr>(&Node)) {
     case UETT_SizeOf:
     case UETT_AlignOf:
       return true;
     default:
       return false;
     }
   }
   if (const auto *TypeIDExpr = dyn_cast<CXXTypeidExpr>(&Node))
-    return !TypeIDExpr->isPotentiallyEvaluated();
+    return !TypeIDExpr->isPotentiallyEvaluated(Finder->getASTContext());
   return false;
 }
 
 /// Contains information about a use-after-move.
 struct UseAfterMove {
   // The DeclRefExpr that constituted the use of the object.
   const DeclRefExpr *DeclRef;
 

clang/include/clang/AST/EvaluatedExprVisitor.h

#define PTR(CLASS) typename Ptr<CLASS>::type
 
   void VisitDesignatedInitExpr(PTR(DesignatedInitExpr) E) {
     // Only the actual initializer matters; the designators are all constant
     // expressions.
     return this->Visit(E->getInit());
   }
 
   void VisitCXXTypeidExpr(PTR(CXXTypeidExpr) E) {
-    if (E->isPotentiallyEvaluated())
+    if (E->isPotentiallyEvaluated(Context))
       return this->Visit(E->getExprOperand());
   }
 
   void VisitCallExpr(PTR(CallExpr) CE) {
     if (!CE->isUnevaluatedBuiltinCall(Context))
       return static_cast<ImplClass*>(this)->VisitExpr(CE);
   }
 

clang/include/clang/AST/ExprCXX.h

 ///
 /// This represents code like \c typeid(int) or \c typeid(*objPtr)
 class CXXTypeidExpr : public Expr {
   friend class ASTStmtReader;
 
 private:
   llvm::PointerUnion<Stmt *, TypeSourceInfo *> Operand;
   SourceRange Range;
+  bool isMSABI;
 
 public:
   CXXTypeidExpr(QualType Ty, TypeSourceInfo *Operand, SourceRange R)
       : Expr(CXXTypeidExprClass, Ty, VK_LValue, OK_Ordinary), Operand(Operand),
-        Range(R) {
+        Range(R), isMSABI(false) {
     setDependence(computeDependence(this));
   }
 
   CXXTypeidExpr(QualType Ty, Expr *Operand, SourceRange R)
       : Expr(CXXTypeidExprClass, Ty, VK_LValue, OK_Ordinary), Operand(Operand),
-        Range(R) {
+        Range(R), isMSABI(false) {
     setDependence(computeDependence(this));
   }
 
   CXXTypeidExpr(EmptyShell Empty, bool isExpr)
       : Expr(CXXTypeidExprClass, Empty) {
     if (isExpr)
       Operand = (Expr*)nullptr;
     else
       Operand = (TypeSourceInfo*)nullptr;
   }
 
   /// Determine whether this typeid has a type operand which is potentially
   /// evaluated, per C++11 [expr.typeid]p3.
-  bool isPotentiallyEvaluated() const;
+  bool isPotentiallyEvaluated(const ASTContext &Context) const;
 
   bool isTypeOperand() const { return Operand.is<TypeSourceInfo *>(); }
 
   /// Retrieves the type operand of this typeid() expression after
   /// various required adjustments (removing reference types, cv-qualifiers).
   QualType getTypeOperand(ASTContext &Context) const;
 
   /// Retrieve source information for the type operand.
   TypeSourceInfo *getTypeOperandSourceInfo() const {
     assert(isTypeOperand() && "Cannot call getTypeOperand for typeid(expr)");

clang/lib/AST/Expr.cpp

if (CE->getCastKind() == CK_LValueToRValue &&
         CE->getSubExpr()->getType().isVolatileQualified())
       return true;
     break;
   }
 
   case CXXTypeidExprClass:
     // typeid might throw if its subexpression is potentially-evaluated, so has
     // side-effects in that case whether or not its subexpression does.
-    return cast<CXXTypeidExpr>(this)->isPotentiallyEvaluated();
+    return cast<CXXTypeidExpr>(this)->isPotentiallyEvaluated(Ctx);
 
   case CXXConstructExprClass:
   case CXXTemporaryObjectExprClass: {
     const CXXConstructExpr *CE = cast<CXXConstructExpr>(this);
     if (!CE->getConstructor()->isTrivial() && IncludePossibleEffects)
       return true;
     // A trivial constructor does not add any side-effects of its own. Just look
     // at its arguments.

clang/lib/AST/ExprCXX.cpp

CXXRewrittenBinaryOperator::getDecomposedForm() const {
   }
 
   // Put the comparison operands in the right order.
   if (isReversed())
     std::swap(Result.LHS, Result.RHS);
   return Result;
 }
 
-bool CXXTypeidExpr::isPotentiallyEvaluated() const {
+bool CXXTypeidExpr::isPotentiallyEvaluated(const ASTContext &Context) const {
   if (isTypeOperand())
     return false;
 
+  Expr *E = getExprOperand()->IgnoreParenNoopCasts(Context);
+  if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+    QualType Ty = DRE->getDecl()->getType();
+    if (!Ty->isPointerType() && !Ty->isReferenceType())
+      return false;
+  }
+
   // C++11 [expr.typeid]p3:
   //   When typeid is applied to an expression other than a glvalue of
   //   polymorphic class type, [...] the expression is an unevaluated operand.
-  const Expr *E = getExprOperand();
   if (const CXXRecordDecl *RD = E->getType()->getAsCXXRecordDecl())
     if (RD->isPolymorphic() && E->isGLValue())
       return true;
 
   return false;
 }
 
 QualType CXXTypeidExpr::getTypeOperand(ASTContext &Context) const {
   assert(isTypeOperand() && "Cannot call getTypeOperand for typeid(expr)");
   Qualifiers Quals;
   return Context.getUnqualifiedArrayType(
       Operand.get<TypeSourceInfo *>()->getType().getNonReferenceType(), Quals);
 }
 
 QualType CXXUuidofExpr::getTypeOperand(ASTContext &Context) const {
   assert(isTypeOperand() && "Cannot call getTypeOperand for __uuidof(expr)");
   Qualifiers Quals;
   return Context.getUnqualifiedArrayType(
       Operand.get<TypeSourceInfo *>()->getType().getNonReferenceType(), Quals);
 }
 

clang/lib/AST/ExprConstant.cpp

LValueExprEvaluator::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
   // Defer visiting the literal until the lvalue-to-rvalue conversion. We can
   // only see this when folding in C, so there's no standard to follow here.
   return Success(E);
 }
 
 bool LValueExprEvaluator::VisitCXXTypeidExpr(const CXXTypeidExpr *E) {
   TypeInfoLValue TypeInfo;
 
-  if (!E->isPotentiallyEvaluated()) {
+  if (!E->isPotentiallyEvaluated(Info.Ctx)) {
     if (E->isTypeOperand())
       TypeInfo = TypeInfoLValue(E->getTypeOperand(Info.Ctx).getTypePtr());
     else
       TypeInfo = TypeInfoLValue(E->getExprOperand()->getType().getTypePtr());
   } else {
     if (!Info.Ctx.getLangOpts().CPlusPlus20) {
       Info.CCEDiag(E, diag::note_constexpr_typeid_polymorphic)
         << E->getExprOperand()->getType()

clang/lib/Analysis/ExprMutationAnalyzer.cpp

AST_MATCHER_P(Expr, maybeEvalCommaExpr,
   }
   return InnerMatcher.matches(*Result, Finder, Builder);
 }
 
 const ast_matchers::internal::VariadicDynCastAllOfMatcher<Stmt, CXXTypeidExpr>
     cxxTypeidExpr;
 
 AST_MATCHER(CXXTypeidExpr, isPotentiallyEvaluated) {
-  return Node.isPotentiallyEvaluated();
+  return Node.isPotentiallyEvaluated(Finder->getASTContext());
 }
 
 const ast_matchers::internal::VariadicDynCastAllOfMatcher<Stmt,
                                                           GenericSelectionExpr>
     genericSelectionExpr;
 
 AST_MATCHER_P(GenericSelectionExpr, hasControllingExpr,
               ast_matchers::internal::Matcher<Expr>, InnerMatcher) {

clang/lib/CodeGen/CGExprCXX.cpp

if (E->isTypeOperand()) {
     return Builder.CreateBitCast(TypeInfo, StdTypeInfoPtrTy);
   }
 
   // C++ [expr.typeid]p2:
   //   When typeid is applied to a glvalue expression whose type is a
   //   polymorphic class type, the result refers to a std::type_info object
   //   representing the type of the most derived object (that is, the dynamic
   //   type) to which the glvalue refers.
-  if (E->isPotentiallyEvaluated())
+  if (E->isPotentiallyEvaluated(getContext()))
     return EmitTypeidFromVTable(*this, E->getExprOperand(),
                                 StdTypeInfoPtrTy);
 
   QualType OperandTy = E->getExprOperand()->getType();
   return Builder.CreateBitCast(CGM.GetAddrOfRTTIDescriptor(OperandTy),
                                StdTypeInfoPtrTy);
 }
 

clang/test/CodeGenCXX/microsoft-abi-typeid.cpp

 // CHECK-NEXT:   ret %struct.type_info* [[RET]]
 
 const std::type_info* test4_typeid() { return &typeid(b); }
 // CHECK: define dso_local %struct.type_info* @"?test4_typeid@@YAPBUtype_info@@XZ"()
 // CHECK:   ret %struct.type_info* bitcast (%rtti.TypeDescriptor2* @"??_R0H@8" to %struct.type_info*)
 
 const std::type_info* test5_typeid() { return &typeid(v); }
 // CHECK: define dso_local %struct.type_info* @"?test5_typeid@@YAPBUtype_info@@XZ"()
-// CHECK:        [[RT:%.*]] = call i8* @__RTtypeid(i8* bitcast (%struct.V* @"?v@@3UV@@A" to i8*))
-// CHECK-NEXT:   [[RET:%.*]] = bitcast i8* [[RT]] to %struct.type_info*
-// CHECK-NEXT:   ret %struct.type_info* [[RET]]
+// CHECK:   ret %struct.type_info* bitcast (%rtti.TypeDescriptor7* @"??_R0?AUV@@@8" to %struct.type_info*)
+
+const std::type_info* test6_typeid() { return &typeid((V&)v); }
+// CHECK: define dso_local %struct.type_info* @"?test6_typeid@@YAPBUtype_info@@XZ"()
+// CHECK:   ret %struct.type_info* bitcast (%rtti.TypeDescriptor7* @"??_R0?AUV@@@8" to %struct.type_info*)
 
 namespace PR26329 {
 struct Polymorphic {
   virtual ~Polymorphic();
 };
 
 void f(const Polymorphic &poly) {
   try {

clang/test/SemaCXX/constant-expression-cxx2a.cpp

namespace TypeId {
   };
   struct B2 : B {
     const std::type_info &ti2 = typeid(*this);
   };
   static_assert(&B2().ti1 == &typeid(B));
   static_assert(&B2().ti2 == &typeid(B2));
   extern B2 extern_b2;
   // expected-note@+1 {{typeid applied to object 'extern_b2' whose dynamic type is not constant}}
-  static_assert(&typeid(extern_b2) == &typeid(B2)); // expected-error {{constant expression}}
+  static_assert(&typeid(*&extern_b2) == &typeid(B2)); // expected-error {{constant expression}}
 
   constexpr B2 b2;
   constexpr const B &b1 = b2;
   static_assert(&typeid(b1) == &typeid(B2));
 
   constexpr bool side_effects() {
     // Not polymorphic nor a glvalue.
     bool OK = true;