Skip to content

Commit

Permalink
Fix clang 13 assertion when building math/frobby
Browse files Browse the repository at this point in the history
Merge commit d9308aa39b23 from llvm git (by Matheus Izvekov):

  [clang] don't mark as Elidable CXXConstruct expressions used in NRVO

  See PR51862.

  The consumers of the Elidable flag in CXXConstructExpr assume that
  an elidable construction just goes through a single copy/move construction,
  so that the source object is immediately passed as an argument and is the same
  type as the parameter itself.

  With the implementation of P2266 and after some adjustments to the
  implementation of P1825, we started (correctly, as per standard)
  allowing more cases where the copy initialization goes through
  user defined conversions.

  With this patch we stop using this flag in NRVO contexts, to preserve code
  that relies on that assumption.
  This causes no known functional changes, we just stop firing some asserts
  in a cople of included test cases.

  Reviewed By: rsmith

  Differential Revision: https://reviews.llvm.org/D109800

PR:		258209
  • Loading branch information
DimitryAndric committed Sep 22, 2021
1 parent 22f561b commit e68c92f
Show file tree
Hide file tree
Showing 11 changed files with 51 additions and 33 deletions.
16 changes: 8 additions & 8 deletions contrib/llvm-project/clang/include/clang/Sema/Initialization.h
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,8 @@ class alignas(8) InitializedEntity {

/// Create the initialization entity for the result of a function.
static InitializedEntity InitializeResult(SourceLocation ReturnLoc,
QualType Type, bool NRVO) {
return InitializedEntity(EK_Result, ReturnLoc, Type, NRVO);
QualType Type) {
return InitializedEntity(EK_Result, ReturnLoc, Type);
}

static InitializedEntity InitializeStmtExprResult(SourceLocation ReturnLoc,
Expand All @@ -308,20 +308,20 @@ class alignas(8) InitializedEntity {
}

static InitializedEntity InitializeBlock(SourceLocation BlockVarLoc,
QualType Type, bool NRVO) {
return InitializedEntity(EK_BlockElement, BlockVarLoc, Type, NRVO);
QualType Type) {
return InitializedEntity(EK_BlockElement, BlockVarLoc, Type);
}

static InitializedEntity InitializeLambdaToBlock(SourceLocation BlockVarLoc,
QualType Type, bool NRVO) {
QualType Type) {
return InitializedEntity(EK_LambdaToBlockConversionBlockElement,
BlockVarLoc, Type, NRVO);
BlockVarLoc, Type);
}

/// Create the initialization entity for an exception object.
static InitializedEntity InitializeException(SourceLocation ThrowLoc,
QualType Type, bool NRVO) {
return InitializedEntity(EK_Exception, ThrowLoc, Type, NRVO);
QualType Type) {
return InitializedEntity(EK_Exception, ThrowLoc, Type);
}

/// Create the initialization entity for an object allocated via new.
Expand Down
15 changes: 12 additions & 3 deletions contrib/llvm-project/clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9934,10 +9934,19 @@ bool RecordExprEvaluator::VisitCXXConstructExpr(const CXXConstructExpr *E,
return false;

// Avoid materializing a temporary for an elidable copy/move constructor.
if (E->isElidable() && !ZeroInit)
if (const MaterializeTemporaryExpr *ME
= dyn_cast<MaterializeTemporaryExpr>(E->getArg(0)))
if (E->isElidable() && !ZeroInit) {
// FIXME: This only handles the simplest case, where the source object
// is passed directly as the first argument to the constructor.
// This should also handle stepping though implicit casts and
// and conversion sequences which involve two steps, with a
// conversion operator followed by a converting constructor.
const Expr *SrcObj = E->getArg(0);
assert(SrcObj->isTemporaryObject(Info.Ctx, FD->getParent()));
assert(Info.Ctx.hasSameUnqualifiedType(E->getType(), SrcObj->getType()));
if (const MaterializeTemporaryExpr *ME =
dyn_cast<MaterializeTemporaryExpr>(SrcObj))
return Visit(ME->getSubExpr());
}

if (ZeroInit && !ZeroInitialization(E, T))
return false;
Expand Down
19 changes: 11 additions & 8 deletions contrib/llvm-project/clang/lib/CodeGen/CGExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -609,15 +609,18 @@ CodeGenFunction::EmitCXXConstructExpr(const CXXConstructExpr *E,
return;

// Elide the constructor if we're constructing from a temporary.
// The temporary check is required because Sema sets this on NRVO
// returns.
if (getLangOpts().ElideConstructors && E->isElidable()) {
assert(getContext().hasSameUnqualifiedType(E->getType(),
E->getArg(0)->getType()));
if (E->getArg(0)->isTemporaryObject(getContext(), CD->getParent())) {
EmitAggExpr(E->getArg(0), Dest);
return;
}
// FIXME: This only handles the simplest case, where the source object
// is passed directly as the first argument to the constructor.
// This should also handle stepping though implicit casts and
// conversion sequences which involve two steps, with a
// conversion operator followed by a converting constructor.
const Expr *SrcObj = E->getArg(0);
assert(SrcObj->isTemporaryObject(getContext(), CD->getParent()));
assert(
getContext().hasSameUnqualifiedType(E->getType(), SrcObj->getType()));
EmitAggExpr(SrcObj, Dest);
return;
}

if (const ArrayType *arrayType
Expand Down
2 changes: 1 addition & 1 deletion contrib/llvm-project/clang/lib/Sema/Sema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2010,7 +2010,7 @@ static void checkEscapingByref(VarDecl *VD, Sema &S) {
Expr *VarRef =
new (S.Context) DeclRefExpr(S.Context, VD, false, T, VK_LValue, Loc);
ExprResult Result;
auto IE = InitializedEntity::InitializeBlock(Loc, T, false);
auto IE = InitializedEntity::InitializeBlock(Loc, T);
if (S.getLangOpts().CPlusPlus2b) {
auto *E = ImplicitCastExpr::Create(S.Context, T, CK_NoOp, VarRef, nullptr,
VK_XValue, FPOptionsOverride());
Expand Down
2 changes: 1 addition & 1 deletion contrib/llvm-project/clang/lib/Sema/SemaCoroutine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1533,7 +1533,7 @@ bool CoroutineStmtBuilder::makeGroDeclAndReturnStmt() {
if (GroType->isVoidType()) {
// Trigger a nice error message.
InitializedEntity Entity =
InitializedEntity::InitializeResult(Loc, FnRetType, false);
InitializedEntity::InitializeResult(Loc, FnRetType);
S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue);
noteMemberDeclaredHere(S, ReturnValue, Fn);
return false;
Expand Down
9 changes: 9 additions & 0 deletions contrib/llvm-project/clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15262,8 +15262,17 @@ Sema::BuildCXXConstructExpr(SourceLocation ConstructLoc, QualType DeclInitType,
// can be omitted by constructing the temporary object
// directly into the target of the omitted copy/move
if (ConstructKind == CXXConstructExpr::CK_Complete && Constructor &&
// FIXME: Converting constructors should also be accepted.
// But to fix this, the logic that digs down into a CXXConstructExpr
// to find the source object needs to handle it.
// Right now it assumes the source object is passed directly as the
// first argument.
Constructor->isCopyOrMoveConstructor() && hasOneRealArgument(ExprArgs)) {
Expr *SubExpr = ExprArgs[0];
// FIXME: Per above, this is also incorrect if we want to accept
// converting constructors, as isTemporaryObject will
// reject temporaries with different type from the
// CXXRecord itself.
Elidable = SubExpr->isTemporaryObject(
Context, cast<CXXRecordDecl>(FoundDecl->getDeclContext()));
}
Expand Down
2 changes: 1 addition & 1 deletion contrib/llvm-project/clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15683,7 +15683,7 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
if (!Result.isInvalid()) {
Result = PerformCopyInitialization(
InitializedEntity::InitializeBlock(Var->getLocation(),
Cap.getCaptureType(), false),
Cap.getCaptureType()),
Loc, Result.get());
}

Expand Down
5 changes: 2 additions & 3 deletions contrib/llvm-project/clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -893,9 +893,8 @@ ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex,
if (CheckCXXThrowOperand(OpLoc, ExceptionObjectTy, Ex))
return ExprError();

InitializedEntity Entity = InitializedEntity::InitializeException(
OpLoc, ExceptionObjectTy,
/*NRVO=*/NRInfo.isCopyElidable());
InitializedEntity Entity =
InitializedEntity::InitializeException(OpLoc, ExceptionObjectTy);
ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRInfo, Ex);
if (Res.isInvalid())
return ExprError();
Expand Down
3 changes: 1 addition & 2 deletions contrib/llvm-project/clang/lib/Sema/SemaLambda.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1975,8 +1975,7 @@ ExprResult Sema::BuildBlockForLambdaConversion(SourceLocation CurrentLocation,
CallOperator->markUsed(Context);

ExprResult Init = PerformCopyInitialization(
InitializedEntity::InitializeLambdaToBlock(ConvLocation, Src->getType(),
/*NRVO=*/false),
InitializedEntity::InitializeLambdaToBlock(ConvLocation, Src->getType()),
CurrentLocation, Src);
if (!Init.isInvalid())
Init = ActOnFinishFullExpr(Init.get(), /*DiscardedValue*/ false);
Expand Down
3 changes: 1 addition & 2 deletions contrib/llvm-project/clang/lib/Sema/SemaObjCProperty.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1467,8 +1467,7 @@ Decl *Sema::ActOnPropertyImplDecl(Scope *S,
LoadSelfExpr, true, true);
ExprResult Res = PerformCopyInitialization(
InitializedEntity::InitializeResult(PropertyDiagLoc,
getterMethod->getReturnType(),
/*NRVO=*/false),
getterMethod->getReturnType()),
PropertyDiagLoc, IvarRefExpr);
if (!Res.isInvalid()) {
Expr *ResExpr = Res.getAs<Expr>();
Expand Down
8 changes: 4 additions & 4 deletions contrib/llvm-project/clang/lib/Sema/SemaStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3653,8 +3653,8 @@ StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc,

// In C++ the return statement is handled via a copy initialization.
// the C version of which boils down to CheckSingleAssignmentConstraints.
InitializedEntity Entity = InitializedEntity::InitializeResult(
ReturnLoc, FnRetType, NRVOCandidate != nullptr);
InitializedEntity Entity =
InitializedEntity::InitializeResult(ReturnLoc, FnRetType);
ExprResult Res = PerformMoveOrCopyInitialization(
Entity, NRInfo, RetValExp, SupressSimplerImplicitMoves);
if (Res.isInvalid()) {
Expand Down Expand Up @@ -4085,8 +4085,8 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
// the C version of which boils down to CheckSingleAssignmentConstraints.
if (!HasDependentReturnType && !RetValExp->isTypeDependent()) {
// we have a non-void function with an expression, continue checking
InitializedEntity Entity = InitializedEntity::InitializeResult(
ReturnLoc, RetType, NRVOCandidate != nullptr);
InitializedEntity Entity =
InitializedEntity::InitializeResult(ReturnLoc, RetType);
ExprResult Res = PerformMoveOrCopyInitialization(
Entity, NRInfo, RetValExp, SupressSimplerImplicitMoves);
if (Res.isInvalid()) {
Expand Down

0 comments on commit e68c92f

Please sign in to comment.