From: Richard Smith Date: Mon, 27 Jan 2014 04:19:56 +0000 (+0000) Subject: PR17052 / DR1560 (+DR1550): In a conditional expression between a glvalue and a X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=908319e459c087ad06abb9d36366fe33f9fe47f9;p=clang PR17052 / DR1560 (+DR1550): In a conditional expression between a glvalue and a throw-expression, the result is also a glvalue and isn't unnecessarily coerced to a prvalue. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@200189 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/AST/ExprClassification.cpp b/lib/AST/ExprClassification.cpp index f9d2b2c778..55b9f13b29 100644 --- a/lib/AST/ExprClassification.cpp +++ b/lib/AST/ExprClassification.cpp @@ -541,10 +541,21 @@ static Cl::Kinds ClassifyConditional(ASTContext &Ctx, const Expr *True, "This is only relevant for C++."); // C++ [expr.cond]p2 - // If either the second or the third operand has type (cv) void, [...] - // the result [...] is a prvalue. - if (True->getType()->isVoidType() || False->getType()->isVoidType()) + // If either the second or the third operand has type (cv) void, + // one of the following shall hold: + if (True->getType()->isVoidType() || False->getType()->isVoidType()) { + // The second or the third operand (but not both) is a (possibly + // parenthesized) throw-expression; the result is of the [...] value + // category of the other. + bool TrueIsThrow = isa(True->IgnoreParenImpCasts()); + bool FalseIsThrow = isa(False->IgnoreParenImpCasts()); + if (const Expr *NonThrow = TrueIsThrow ? (FalseIsThrow ? 0 : False) + : (FalseIsThrow ? True : 0)) + return ClassifyInternal(Ctx, NonThrow); + + // [Otherwise] the result [...] is a prvalue. return Cl::CL_PRValue; + } // Note that at this point, we have already performed all conversions // according to [expr.cond]p3. diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 3e6b39a09b..0dffa53679 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -4185,20 +4185,21 @@ static Expr *EvalAddr(Expr *E, SmallVectorImpl &refVars, ConditionalOperator *C = cast(E); // Handle the GNU extension for missing LHS. - if (Expr *lhsExpr = C->getLHS()) { - // In C++, we can have a throw-expression, which has 'void' type. - if (!lhsExpr->getType()->isVoidType()) - if (Expr* LHS = EvalAddr(lhsExpr, refVars, ParentDecl)) + // FIXME: That isn't a ConditionalOperator, so doesn't get here. + if (Expr *LHSExpr = C->getLHS()) { + // In C++, we can have a throw-expression, which has 'void' type. + if (!LHSExpr->getType()->isVoidType()) + if (Expr *LHS = EvalAddr(LHSExpr, refVars, ParentDecl)) return LHS; } // In C++, we can have a throw-expression, which has 'void' type. if (C->getRHS()->getType()->isVoidType()) - return NULL; + return 0; return EvalAddr(C->getRHS(), refVars, ParentDecl); } - + case Stmt::BlockExprClass: if (cast(E)->getBlockDecl()->hasCaptures()) return E; // local block. @@ -4338,9 +4339,16 @@ do { ConditionalOperator *C = cast(E); // Handle the GNU extension for missing LHS. - if (Expr *lhsExpr = C->getLHS()) - if (Expr *LHS = EvalVal(lhsExpr, refVars, ParentDecl)) - return LHS; + if (Expr *LHSExpr = C->getLHS()) { + // In C++, we can have a throw-expression, which has 'void' type. + if (!LHSExpr->getType()->isVoidType()) + if (Expr *LHS = EvalVal(LHSExpr, refVars, ParentDecl)) + return LHS; + } + + // In C++, we can have a throw-expression, which has 'void' type. + if (C->getRHS()->getType()->isVoidType()) + return 0; return EvalVal(C->getRHS(), refVars, ParentDecl); } diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index e71b5ba3ca..2fd3af4550 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/lib/Sema/SemaExprCXX.cpp @@ -4399,42 +4399,21 @@ QualType Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS, bool LVoid = LTy->isVoidType(); bool RVoid = RTy->isVoidType(); if (LVoid || RVoid) { - // ... then the [l2r] conversions are performed on the second and third - // operands ... - LHS = DefaultFunctionArrayLvalueConversion(LHS.take()); - RHS = DefaultFunctionArrayLvalueConversion(RHS.take()); - if (LHS.isInvalid() || RHS.isInvalid()) - return QualType(); - - // Finish off the lvalue-to-rvalue conversion by copy-initializing a - // temporary if necessary. DefaultFunctionArrayLvalueConversion doesn't - // do this part for us. - ExprResult &NonVoid = LVoid ? RHS : LHS; - if (NonVoid.get()->getType()->isRecordType() && - NonVoid.get()->isGLValue()) { - if (RequireNonAbstractType(QuestionLoc, NonVoid.get()->getType(), - diag::err_allocation_of_abstract_type)) - return QualType(); - InitializedEntity Entity = - InitializedEntity::InitializeTemporary(NonVoid.get()->getType()); - NonVoid = PerformCopyInitialization(Entity, SourceLocation(), NonVoid); - if (NonVoid.isInvalid()) - return QualType(); + // ... one of the following shall hold: + // -- The second or the third operand (but not both) is a (possibly + // parenthesized) throw-expression; the result is of the type + // and value category of the other. + bool LThrow = isa(LHS.get()->IgnoreParenImpCasts()); + bool RThrow = isa(RHS.get()->IgnoreParenImpCasts()); + if (LThrow != RThrow) { + Expr *NonThrow = LThrow ? RHS.get() : LHS.get(); + VK = NonThrow->getValueKind(); + // DR (no number yet): the result is a bit-field if the + // non-throw-expression operand is a bit-field. + OK = NonThrow->getObjectKind(); + return NonThrow->getType(); } - LTy = LHS.get()->getType(); - RTy = RHS.get()->getType(); - - // ... and one of the following shall hold: - // -- The second or the third operand (but not both) is a throw- - // expression; the result is of the type of the other and is a prvalue. - bool LThrow = isa(LHS.get()->IgnoreParenCasts()); - bool RThrow = isa(RHS.get()->IgnoreParenCasts()); - if (LThrow && !RThrow) - return RTy; - if (RThrow && !LThrow) - return LTy; - // -- Both the second and third operands have type void; the result is of // type void and is a prvalue. if (LVoid && RVoid) diff --git a/test/CXX/drs/dr15xx.cpp b/test/CXX/drs/dr15xx.cpp new file mode 100644 index 0000000000..66618c1716 --- /dev/null +++ b/test/CXX/drs/dr15xx.cpp @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -std=c++98 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors +// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors +// RUN: %clang_cc1 -std=c++1y -triple x86_64-unknown-unknown %s -verify -fexceptions -fcxx-exceptions -pedantic-errors + +// expected-no-diagnostics + +namespace DR1550 { // dr1550: yes + int f(bool b, int n) { + return (b ? (throw 0) : n) + (b ? n : (throw 0)); + } +} + +namespace DR1560 { // dr1560: 3.5 + void f(bool b, int n) { + (b ? throw 0 : n) = (b ? n : throw 0) = 0; + } + class X { X(const X&); }; + const X &get(); + const X &x = true ? get() : throw 0; +} diff --git a/test/CodeGenCXX/throw-expressions.cpp b/test/CodeGenCXX/throw-expressions.cpp index d9bf8fdd59..7e81141d79 100644 --- a/test/CodeGenCXX/throw-expressions.cpp +++ b/test/CodeGenCXX/throw-expressions.cpp @@ -67,3 +67,16 @@ int test6(bool x, bool y, int z) { // // end: // CHECK: ret i32 + +namespace DR1560 { + struct A { + ~A(); + }; + extern bool b; + A get(); + // CHECK-LABEL: @_ZN6DR15601bE + const A &r = b ? get() : throw 0; + // CHECK-NOT: call {{.*}}@_ZN6DR15601AD1Ev + // CHECK: call {{.*}} @__cxa_atexit({{.*}} @_ZN6DR15601AD1Ev {{.*}} @_ZGRN6DR15601rE + // CHECK-NOT: call {{.*}}@_ZN6DR15601AD1Ev +} diff --git a/test/SemaCXX/conditional-expr.cpp b/test/SemaCXX/conditional-expr.cpp index 5abee4a3c4..538de5847d 100644 --- a/test/SemaCXX/conditional-expr.cpp +++ b/test/SemaCXX/conditional-expr.cpp @@ -75,6 +75,7 @@ void test() int i1 = ToBool() ? 0 : 1; // p2 (one or both void, and throwing) + Fields flds; i1 ? throw 0 : throw 1; i1 ? test() : throw 1; i1 ? throw 0 : test(); @@ -85,8 +86,16 @@ void test() i1 = i1 ? 0 : (throw 0); i1 ? 0 : test(); // expected-error {{right operand to ? is void, but left operand is of type 'int'}} i1 ? test() : 0; // expected-error {{left operand to ? is void, but right operand is of type 'int'}} - (i1 ? throw 0 : i1) = 0; // expected-error {{expression is not assignable}} - (i1 ? i1 : throw 0) = 0; // expected-error {{expression is not assignable}} + (i1 ? throw 0 : i1) = 0; + (i1 ? i1 : throw 0) = 0; + (i1 ? (throw 0) : i1) = 0; + (i1 ? i1 : (throw 0)) = 0; + (i1 ? (void)(throw 0) : i1) = 0; // expected-error {{left operand to ? is void, but right operand is of type 'int'}} + (i1 ? i1 : (void)(throw 0)) = 0; // expected-error {{right operand to ? is void, but left operand is of type 'int'}} + int &throwRef1 = (i1 ? flds.i1 : throw 0); + int &throwRef2 = (i1 ? throw 0 : flds.i1); + int &throwRef3 = (i1 ? flds.b1 : throw 0); // expected-error {{non-const reference cannot bind to bit-field}} + int &throwRef4 = (i1 ? throw 0 : flds.b1); // expected-error {{non-const reference cannot bind to bit-field}} // p3 (one or both class type, convert to each other) // b1 (lvalues) @@ -151,7 +160,6 @@ void test() &(i1 ? i1 : i2); // expected-error {{cannot take the address of an rvalue}} // p4 (lvalue, same type) - Fields flds; int &ir1 = i1 ? flds.i1 : flds.i2; (i1 ? flds.b1 : flds.i2) = 0; (i1 ? flds.i1 : flds.b2) = 0; @@ -219,8 +227,8 @@ void test() // *must* create a separate temporary copy of class objects. This can only // be properly tested at runtime, though. - const Abstract &a = true ? static_cast(Derived1()) : Derived2(); // expected-error {{allocating an object of abstract class type 'const Abstract'}} - true ? static_cast(Derived1()) : throw 3; // expected-error {{allocating an object of abstract class type 'const Abstract'}} + const Abstract &abstract1 = true ? static_cast(Derived1()) : Derived2(); // expected-error {{allocating an object of abstract class type 'const Abstract'}} + const Abstract &abstract2 = true ? static_cast(Derived1()) : throw 3; // ok } namespace PR6595 { @@ -367,3 +375,12 @@ namespace DR587 { const volatile int &cvir2 = b ? cvi : vi; const volatile int &cvir3 = b ? ci : vi; // expected-error{{volatile lvalue reference to type 'const volatile int' cannot bind to a temporary of type 'int'}} } + +namespace PR17052 { + struct X { + int i_; + bool b_; + + int &test() { return b_ ? i_ : throw 1; } + }; +} diff --git a/test/SemaCXX/expression-traits.cpp b/test/SemaCXX/expression-traits.cpp index 3a00687f11..51bb90e8bb 100644 --- a/test/SemaCXX/expression-traits.cpp +++ b/test/SemaCXX/expression-traits.cpp @@ -520,20 +520,19 @@ void expr_cond(bool cond) // 5.16 Conditional operator [expr.cond] // // 2 If either the second or the third operand has type (possibly - // cv-qualified) void, then the lvalue-to-rvalue (4.1), - // array-to-pointer (4.2), and function-to-pointer (4.3) standard - // conversions are performed on the second and third operands, and one - // of the following shall hold: + // cv-qualified) void, one of the following shall hold: // // - The second or the third operand (but not both) is a - // throw-expression (15.1); the result is of the type of the other and - // is an rvalue. + // (possibly parenthesized) throw-expression (15.1); the result + // is of the type and value category of the other. Class classLvalue; ASSERT_RVALUE(cond ? throw 1 : (void)0); ASSERT_RVALUE(cond ? (void)0 : throw 1); - ASSERT_RVALUE(cond ? throw 1 : classLvalue); - ASSERT_RVALUE(cond ? classLvalue : throw 1); + ASSERT_RVALUE(cond ? throw 1 : 0); + ASSERT_RVALUE(cond ? 0 : throw 1); + ASSERT_LVALUE(cond ? throw 1 : classLvalue); + ASSERT_LVALUE(cond ? classLvalue : throw 1); // - Both the second and the third operands have type void; the result // is of type void and is an rvalue. [Note: this includes the case diff --git a/www/cxx_dr_status.html b/www/cxx_dr_status.html index 8fa4b97f9b..e2da255f13 100644 --- a/www/cxx_dr_status.html +++ b/www/cxx_dr_status.html @@ -9115,7 +9115,7 @@ and POD class 1550 DRWP Parenthesized throw-expression operand of conditional-expression - Unknown + Yes 1551 @@ -9175,7 +9175,7 @@ and POD class 1560 DRWP Gratuitous lvalue-to-rvalue conversion in conditional-expression with throw-expression operand - Unknown + SVN 1561