Commit d7b669b3 authored by Matheus Izvekov's avatar Matheus Izvekov Committed by Tom Stellard
Browse files

[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

(cherry picked from commit d9308aa3)
parent ee6913cc

clang/include/clang/Sema/Initialization.h

+8 −8
Original line number Diff line number Diff line
@@ -298,8 +298,8 @@ public: 


  /// 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,
@@ -308,20 +308,20 @@ public: 
  }



  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.

clang/lib/AST/ExprConstant.cpp

+12 −3
Original line number Diff line number Diff line
@@ -9931,10 +9931,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;

































clang/lib/CodeGen/CGExprCXX.cpp



+11
−8






























Original line number
Diff line number
Diff line








@@ -609,16 +609,19 @@ 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);













    // 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















        = getContext().getAsArrayType(E->getType())) {

































clang/lib/Sema/Sema.cpp



+1
−1






























Original line number
Diff line number
Diff line








@@ -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());

































clang/lib/Sema/SemaCoroutine.cpp



+1
−1






























Original line number
Diff line number
Diff line








@@ -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;