From b3189a1802e493fd819b766c2051bda04f18617c Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Mon, 5 Dec 2016 07:49:14 +0000 Subject: [PATCH] DR1213: element access on an array xvalue or prvalue produces an xvalue. In the latter case, a temporary array object is materialized, and can be lifetime-extended by binding a reference to the member access. Likewise, in an array-to-pointer decay, an rvalue array is materialized before being converted into a pointer. This caused IR generation to stop treating file-scope array compound literals as having static storage duration in some cases in C++; that has been rectified by modeling such a compound literal as an lvalue. This also improves clang's compatibility with GCC for those cases. llvm-svn: 288654 --- clang/lib/AST/ExprClassification.cpp | 18 +++++++---- clang/lib/AST/ExprConstant.cpp | 6 ++-- clang/lib/Sema/Sema.cpp | 12 ++++++++ clang/lib/Sema/SemaExpr.cpp | 38 ++++++++++++++++++++---- clang/lib/Sema/SemaInit.cpp | 7 +++-- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 10 +------ clang/test/Analysis/explain-svals.cpp | 2 +- clang/test/CXX/drs/dr12xx.cpp | 11 +++++++ clang/test/CodeGenCXX/compound-literals.cpp | 24 ++++++++++----- clang/test/CodeGenCXX/stack-reuse.cpp | 2 +- clang/test/CodeGenCXX/temporaries.cpp | 30 +++++++++++++++++++ clang/test/SemaCXX/constant-expression-cxx11.cpp | 27 ++++++++++++----- clang/www/cxx_dr_status.html | 2 +- 13 files changed, 147 insertions(+), 42 deletions(-) diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp index 8388013..aa4651d 100644 --- a/clang/lib/AST/ExprClassification.cpp +++ b/clang/lib/AST/ExprClassification.cpp @@ -141,10 +141,9 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) { return Cl::CL_LValue; // C99 6.5.2.5p5 says that compound literals are lvalues. - // In C++, they're prvalue temporaries. + // In C++, they're prvalue temporaries, except for file-scope arrays. case Expr::CompoundLiteralExprClass: - return Ctx.getLangOpts().CPlusPlus ? ClassifyTemporary(E->getType()) - : Cl::CL_LValue; + return !E->isLValue() ? ClassifyTemporary(E->getType()) : Cl::CL_LValue; // Expressions that are prvalues. case Expr::CXXBoolLiteralExprClass: @@ -196,11 +195,20 @@ static Cl::Kinds ClassifyInternal(ASTContext &Ctx, const Expr *E) { return ClassifyInternal(Ctx, cast(E)->getReplacement()); - // C++ [expr.sub]p1: The result is an lvalue of type "T". - // However, subscripting vector types is more like member access. + // C, C++98 [expr.sub]p1: The result is an lvalue of type "T". + // C++11 (DR1213): in the case of an array operand, the result is an lvalue + // if that operand is an lvalue and an xvalue otherwise. + // Subscripting vector types is more like member access. case Expr::ArraySubscriptExprClass: if (cast(E)->getBase()->getType()->isVectorType()) return ClassifyInternal(Ctx, cast(E)->getBase()); + if (Lang.CPlusPlus11) { + // Step over the array-to-pointer decay if present, but not over the + // temporary materialization. + auto *Base = cast(E)->getBase()->IgnoreImpCasts(); + if (Base->getType()->isArrayType()) + return ClassifyInternal(Ctx, Base); + } return Cl::CL_LValue; // C++ [expr.prim.general]p3: The result is an lvalue if the entity is a diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index e212928..14214bd 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -2907,7 +2907,6 @@ static bool handleLValueToRValueConversion(EvalInfo &Info, const Expr *Conv, // In C99, a CompoundLiteralExpr is an lvalue, and we defer evaluating the // initializer until now for such expressions. Such an expression can't be // an ICE in C, so this only matters for fold. - assert(!Info.getLangOpts().CPlusPlus && "lvalue compound literal in c++?"); if (Type.isVolatileQualified()) { Info.FFDiag(Conv); return false; @@ -4711,7 +4710,7 @@ public: // * VarDecl // * FunctionDecl // - Literals -// * CompoundLiteralExpr in C +// * CompoundLiteralExpr in C (and in global scope in C++) // * StringLiteral // * CXXTypeidExpr // * PredefinedExpr @@ -4906,7 +4905,8 @@ bool LValueExprEvaluator::VisitMaterializeTemporaryExpr( bool LValueExprEvaluator::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) { - assert(!Info.getLangOpts().CPlusPlus && "lvalue compound literal in c++?"); + assert((!Info.getLangOpts().CPlusPlus || E->isFileScope()) && + "lvalue compound literal in c++?"); // 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); diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index fd1a94e..fc76a86 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -390,6 +390,18 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty, if (ExprTy == TypeTy) return E; + // C++1z [conv.array]: The temporary materialization conversion is applied. + // We also use this to fuel C++ DR1213, which applies to C++11 onwards. + if (Kind == CK_ArrayToPointerDecay && getLangOpts().CPlusPlus && + E->getValueKind() == VK_RValue) { + // The temporary is an lvalue in C++98 and an xvalue otherwise. + ExprResult Materialized = CreateMaterializeTemporaryExpr( + E->getType(), E, !getLangOpts().CPlusPlus11); + if (Materialized.isInvalid()) + return ExprError(); + E = Materialized.get(); + } + if (ImplicitCastExpr *ImpCast = dyn_cast(E)) { if (ImpCast->getCastKind() == Kind && (!BasePath || BasePath->empty())) { ImpCast->setType(Ty); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 4415100..5182e16 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -4376,6 +4376,16 @@ Sema::CreateBuiltinArraySubscriptExpr(Expr *Base, SourceLocation LLoc, Expr *LHSExp = Base; Expr *RHSExp = Idx; + ExprValueKind VK = VK_LValue; + ExprObjectKind OK = OK_Ordinary; + + // Per C++ core issue 1213, the result is an xvalue if either operand is + // a non-lvalue array, and an lvalue otherwise. + if (getLangOpts().CPlusPlus11 && + ((LHSExp->getType()->isArrayType() && !LHSExp->isLValue()) || + (RHSExp->getType()->isArrayType() && !RHSExp->isLValue()))) + VK = VK_XValue; + // Perform default conversions. if (!LHSExp->getType()->getAs()) { ExprResult Result = DefaultFunctionArrayLvalueConversion(LHSExp); @@ -4389,8 +4399,6 @@ Sema::CreateBuiltinArraySubscriptExpr(Expr *Base, SourceLocation LLoc, RHSExp = Result.get(); QualType LHSTy = LHSExp->getType(), RHSTy = RHSExp->getType(); - ExprValueKind VK = VK_LValue; - ExprObjectKind OK = OK_Ordinary; // C99 6.5.2.1p2: the expression e1[e2] is by definition precisely equivalent // to the expression *((e1)+(e2)). This means the array "Base" may actually be @@ -5587,11 +5595,31 @@ Sema::BuildCompoundLiteralExpr(SourceLocation LParenLoc, TypeSourceInfo *TInfo, } // In C, compound literals are l-values for some reason. - ExprValueKind VK = getLangOpts().CPlusPlus ? VK_RValue : VK_LValue; + // For GCC compatibility, in C++, file-scope array compound literals with + // constant initializers are also l-values, and compound literals are + // otherwise prvalues. + // + // (GCC also treats C++ list-initialized file-scope array prvalues with + // constant initializers as l-values, but that's non-conforming, so we don't + // follow it there.) + // + // FIXME: It would be better to handle the lvalue cases as materializing and + // lifetime-extending a temporary object, but our materialized temporaries + // representation only supports lifetime extension from a variable, not "out + // of thin air". + // FIXME: For C++, we might want to instead lifetime-extend only if a pointer + // is bound to the result of applying array-to-pointer decay to the compound + // literal. + // FIXME: GCC supports compound literals of reference type, which should + // obviously have a value kind derived from the kind of reference involved. + ExprValueKind VK = + (getLangOpts().CPlusPlus && !(isFileScope && literalType->isArrayType())) + ? VK_RValue + : VK_LValue; return MaybeBindToTemporary( - new (Context) CompoundLiteralExpr(LParenLoc, TInfo, literalType, - VK, LiteralExpr, isFileScope)); + new (Context) CompoundLiteralExpr(LParenLoc, TInfo, literalType, + VK, LiteralExpr, isFileScope)); } ExprResult diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 3937aad..c21dbc1 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -5971,9 +5971,10 @@ performReferenceExtension(Expr *Init, if (CE->getSubExpr()->isGLValue()) Init = CE->getSubExpr(); - // FIXME: Per DR1213, subscripting on an array temporary produces an xvalue. - // It's unclear if binding a reference to that xvalue extends the array - // temporary. + // Per the current approach for DR1299, look through array element access + // when performing lifetime extension. + if (auto *ASE = dyn_cast(Init)) + Init = ASE->getBase(); } while (Init != Old); if (MaterializeTemporaryExpr *ME = dyn_cast(Init)) { diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp index 0f40739..cae6ea0 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -480,15 +480,7 @@ void ExprEngine::VisitCompoundLiteralExpr(const CompoundLiteralExpr *CL, Loc CLLoc = State->getLValue(CL, LCtx); State = State->bindLoc(CLLoc, V); - // Compound literal expressions are a GNU extension in C++. - // Unlike in C, where CLs are lvalues, in C++ CLs are prvalues, - // and like temporary objects created by the functional notation T() - // CLs are destroyed at the end of the containing full-expression. - // HOWEVER, an rvalue of array type is not something the analyzer can - // reason about, since we expect all regions to be wrapped in Locs. - // So we treat array CLs as lvalues as well, knowing that they will decay - // to pointers as soon as they are used. - if (CL->isGLValue() || CL->getType()->isArrayType()) + if (CL->isGLValue()) V = CLLoc; } diff --git a/clang/test/Analysis/explain-svals.cpp b/clang/test/Analysis/explain-svals.cpp index ef2ebc2..be2f830 100644 --- a/clang/test/Analysis/explain-svals.cpp +++ b/clang/test/Analysis/explain-svals.cpp @@ -76,7 +76,7 @@ void test_4(int x, int y) { clang_analyzer_explain(&stat); // expected-warning-re{{{{^pointer to static local variable 'stat'$}}}} clang_analyzer_explain(stat_glob); // expected-warning-re{{{{^initial value of global variable 'stat_glob'$}}}} clang_analyzer_explain(&stat_glob); // expected-warning-re{{{{^pointer to global variable 'stat_glob'$}}}} - clang_analyzer_explain((int[]){1, 2, 3}); // expected-warning-re{{{{^pointer to element of type 'int' with index 0 of compound literal \(int \[3\]\)\{1, 2, 3\}$}}}} + clang_analyzer_explain((int[]){1, 2, 3}); // expected-warning-re{{{{^pointer to element of type 'int' with index 0 of temporary object constructed at statement '\(int \[3\]\)\{1, 2, 3\}'$}}}} } namespace { diff --git a/clang/test/CXX/drs/dr12xx.cpp b/clang/test/CXX/drs/dr12xx.cpp index 048c21a..c4f980a 100644 --- a/clang/test/CXX/drs/dr12xx.cpp +++ b/clang/test/CXX/drs/dr12xx.cpp @@ -5,6 +5,17 @@ // expected-no-diagnostics +namespace dr1213 { // dr1213: 4.0 +#if __cplusplus >= 201103L + using T = int[3]; + int &&r = T{}[1]; + + using T = decltype((T{})); + using U = decltype((T{}[2])); + using U = int &&; +#endif +} + namespace dr1250 { // dr1250: 3.9 struct Incomplete; diff --git a/clang/test/CodeGenCXX/compound-literals.cpp b/clang/test/CodeGenCXX/compound-literals.cpp index 0169d5b..a9882bc 100644 --- a/clang/test/CodeGenCXX/compound-literals.cpp +++ b/clang/test/CodeGenCXX/compound-literals.cpp @@ -12,6 +12,9 @@ struct Y { X x; }; +// CHECK: @.compoundliteral = internal global [5 x i32] [i32 1, i32 2, i32 3, i32 4, i32 5], align 4 +// CHECK: @q = global i32* getelementptr inbounds ([5 x i32], [5 x i32]* @.compoundliteral, i32 0, i32 0), align 4 + // CHECK-LABEL: define i32 @_Z1fv() int f() { // CHECK: [[LVALUE:%[a-z0-9.]+]] = alloca @@ -51,20 +54,27 @@ int *p = (Z){ {1, 2, 3} }.i; // CHECK: store i32* %{{.*}}, i32** @p int *q = (int [5]){1, 2, 3, 4, 5}; +// (constant initialization, checked above) + +extern int n; +int *r = (int [5]){1, 2, 3, 4, 5} + n; // CHECK-LABEL: define {{.*}}__cxx_global_var_init.1() -// CHECK: store i32* getelementptr inbounds ([5 x i32], [5 x i32]* @.compoundliteral, i32 0, i32 0), i32** @q +// CHECK: %[[PTR:.*]] = getelementptr inbounds i32, i32* getelementptr inbounds ([5 x i32], [5 x i32]* @.compoundliteral.2, i32 0, i32 0), i32 % +// CHECK: store i32* %[[PTR]], i32** @r -int *PR21912_1 = (int []){}; -// CHECK-LABEL: define {{.*}}__cxx_global_var_init.2() -// CHECK: store i32* getelementptr inbounds ([0 x i32], [0 x i32]* @.compoundliteral.3, i32 0, i32 0), i32** @PR21912_1 +int *PR21912_1 = (int []){} + n; +// CHECK-LABEL: define {{.*}}__cxx_global_var_init.3() +// CHECK: %[[PTR:.*]] = getelementptr inbounds i32, i32* getelementptr inbounds ([0 x i32], [0 x i32]* @.compoundliteral.4, i32 0, i32 0), i32 % +// CHECK: store i32* %[[PTR]], i32** @PR21912_1 union PR21912Ty { long long l; double d; }; -union PR21912Ty *PR21912_2 = (union PR21912Ty[]){{.d = 2.0}, {.l = 3}}; -// CHECK-LABEL: define {{.*}}__cxx_global_var_init.4() -// CHECK: store %union.PR21912Ty* getelementptr inbounds ([2 x %union.PR21912Ty], [2 x %union.PR21912Ty]* bitcast (<{ { double }, %union.PR21912Ty }>* @.compoundliteral.5 to [2 x %union.PR21912Ty]*), i32 0, i32 0), %union.PR21912Ty** @PR21912_2 +union PR21912Ty *PR21912_2 = (union PR21912Ty[]){{.d = 2.0}, {.l = 3}} + n; +// CHECK-LABEL: define {{.*}}__cxx_global_var_init.5() +// CHECK: %[[PTR:.*]] = getelementptr inbounds %union.PR21912Ty, %union.PR21912Ty* getelementptr inbounds ([2 x %union.PR21912Ty], [2 x %union.PR21912Ty]* bitcast (<{ { double }, %union.PR21912Ty }>* @.compoundliteral.6 to [2 x %union.PR21912Ty]*), i32 0, i32 0), i32 % +// CHECK: store %union.PR21912Ty* %[[PTR]], %union.PR21912Ty** @PR21912_2, align 4 // This compound literal should have local scope. int computed_with_lambda = [] { diff --git a/clang/test/CodeGenCXX/stack-reuse.cpp b/clang/test/CodeGenCXX/stack-reuse.cpp index d6340ef..8325604 100644 --- a/clang/test/CodeGenCXX/stack-reuse.cpp +++ b/clang/test/CodeGenCXX/stack-reuse.cpp @@ -132,8 +132,8 @@ void large_auto_object() { int large_combiner_test(S_large s) { // CHECK-LABEL: define i32 @large_combiner_test -// CHECK: [[T1:%.*]] = alloca %struct.Combiner // CHECK: [[T2:%.*]] = alloca %struct.Combiner +// CHECK: [[T1:%.*]] = alloca %struct.Combiner // CHECK: [[T3:%.*]] = call %struct.Combiner* @_ZN8CombinerC1E7S_large(%struct.Combiner* nonnull [[T1]], [9 x i32] %s.coerce) // CHECK: call void @_ZN8Combiner1fEv(%struct.Combiner* nonnull sret [[T2]], %struct.Combiner* nonnull [[T1]]) // CHECK: [[T4:%.*]] = getelementptr inbounds %struct.Combiner, %struct.Combiner* [[T2]], i32 0, i32 0, i32 0, i32 0 diff --git a/clang/test/CodeGenCXX/temporaries.cpp b/clang/test/CodeGenCXX/temporaries.cpp index c537124..bad51ba 100644 --- a/clang/test/CodeGenCXX/temporaries.cpp +++ b/clang/test/CodeGenCXX/temporaries.cpp @@ -779,6 +779,36 @@ namespace MultipleExtension { } } +namespace ArrayAccess { + struct A { A(int); ~A(); }; + void g(); + void f() { + using T = A[3]; + + // CHECK: call void @_ZN11ArrayAccess1AC1Ei({{.*}}, i32 1 + // CHECK-NOT: @_ZN11ArrayAccess1AD + // CHECK: call void @_ZN11ArrayAccess1AC1Ei({{.*}}, i32 2 + // CHECK-NOT: @_ZN11ArrayAccess1AD + // CHECK: call void @_ZN11ArrayAccess1AC1Ei({{.*}}, i32 3 + // CHECK-NOT: @_ZN11ArrayAccess1AD + A &&a = T{1, 2, 3}[1]; + + // CHECK: call void @_ZN11ArrayAccess1AC1Ei({{.*}}, i32 4 + // CHECK-NOT: @_ZN11ArrayAccess1AD + // CHECK: call void @_ZN11ArrayAccess1AC1Ei({{.*}}, i32 5 + // CHECK-NOT: @_ZN11ArrayAccess1AD + // CHECK: call void @_ZN11ArrayAccess1AC1Ei({{.*}}, i32 6 + // CHECK-NOT: @_ZN11ArrayAccess1AD + A &&b = 2[T{4, 5, 6}]; + + // CHECK: call void @_ZN11ArrayAccess1gEv( + g(); + + // CHECK: call void @_ZN11ArrayAccess1AD + // CHECK: call void @_ZN11ArrayAccess1AD + } +} + namespace PR14130 { struct S { S(int); }; struct U { S &&s; }; diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp index 00bf5bd..8ecf4a4 100644 --- a/clang/test/SemaCXX/constant-expression-cxx11.cpp +++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp @@ -1467,13 +1467,26 @@ namespace VLASizeof { } namespace CompoundLiteral { - // FIXME: - // We don't model the semantics of this correctly: the compound literal is - // represented as a prvalue in the AST, but actually behaves like an lvalue. - // We treat the compound literal as a temporary and refuse to produce a - // pointer to it. This is OK: we're not required to treat this as a constant - // in C++, and in C we model compound literals as lvalues. - constexpr int *p = (int*)(int[1]){0}; // expected-warning {{C99}} expected-error {{constant expression}} expected-note 2{{temporary}} + // Matching GCC, file-scope array compound literals initialized by constants + // are lifetime-extended. + constexpr int *p = (int*)(int[1]){3}; // expected-warning {{C99}} + static_assert(*p == 3, ""); + static_assert((int[2]){1, 2}[1] == 2, ""); // expected-warning {{C99}} + + // Other kinds are not. + struct X { int a[2]; }; + constexpr int *n = (X){1, 2}.a; // expected-warning {{C99}} expected-warning {{temporary array}} + // expected-error@-1 {{constant expression}} + // expected-note@-2 {{pointer to subobject of temporary}} + // expected-note@-3 {{temporary created here}} + + void f() { + static constexpr int *p = (int*)(int[1]){3}; // expected-warning {{C99}} + // expected-error@-1 {{constant expression}} + // expected-note@-2 {{pointer to subobject of temporary}} + // expected-note@-3 {{temporary created here}} + static_assert((int[2]){1, 2}[1] == 2, ""); // expected-warning {{C99}} + } } namespace Vector { diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html index a73a5f6..f144d06 100644 --- a/clang/www/cxx_dr_status.html +++ b/clang/www/cxx_dr_status.html @@ -7093,7 +7093,7 @@ and POD class 1213 CD3 Array subscripting and xvalues - Unknown + SVN 1214 -- 2.7.4