erichkeane added inline comments.
================ Comment at: clang/lib/Sema/SemaConcept.cpp:377-380 + SubstitutedExpression = ImplicitCastExpr::Create( + S.Context, SubstitutedExpression.get()->getType(), + CK_LValueToRValue, SubstitutedExpression.get(), + /*BasePath=*/nullptr, VK_PRValue, FPOptionsOverride()); ---------------- tahonermann wrote: > erichkeane wrote: > > tahonermann wrote: > > > `ImplicitCastExpr::Create()` has the following assertion. I wonder if we > > > need to guard against this here. > > > clang/lib/AST/Expr.cpp: > > > 2073 assert((Kind != CK_LValueToRValue || > > > 2074 !(T->isNullPtrType() || T->getAsCXXRecordDecl())) && > > > 2075 "invalid type for lvalue-to-rvalue conversion"); > > > > > > Or perhaps it would be better to call `Sema::DefaultLvalueConversion()` > > > here? > > I don't think we want all the baggage that DefaultLvalueConversion gets us, > > including diagnostics/etc. That assert IS concerning though... I presume > > we can come up with something to hit that. > > > > It DOES look like that DefeaultLValue conversion replaces the nullptrtypes > > with a NullToPointer cast, so perhaps I should do that, and doesn't do > > anything with a record type. > I'm surprised the additional checks for `nullptr` aren't tripping that > assert; it looks to me like it should. Any ideas? I moved the creation of this AFTER 'CheckConstraintExpression', which will fail unless the type is 'bool' already. SO the expression here cannot be a nullptr or struct type. Before I moved it and wrote the new tests (NullTy and StructTy), it actually did hit the assert you mentioned. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141954/new/ https://reviews.llvm.org/D141954 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
