r181212 - Fix representation of compound literals for C++ objects with destructors.
Richard Smith
richard at metafoo.co.uk
Mon May 6 11:05:58 PDT 2013
On Mon, May 6, 2013 at 10:55 AM, Jordan Rose <jordan_rose at apple.com> wrote:
> Hm, I don't know. I didn't audit CodeGen at all because I was worried
> about destabilizing things on the day of the branch, but I would like the
> simpler AST and subsequent analyzer fix to make it into 3.3.
>
> Obviously we should never have two CXXBindTemporaryExprs for the same
> temporary, but I really don't know if this was the last/only case where we
> did that by accident.
>
Yes, this is not a good thing to change today. It would be interesting to
add an assert to CodeGen and run it through the tests, though, in case
there's more low-hanging fruit here.
> Jordan
>
>
> On May 6, 2013, at 10:35 , Richard Smith <richard at metafoo.co.uk> wrote:
>
> CodeGen has a hack to deal with the possibility of multiple
> CXXBindTemporaryExprs binding to the same temporary. Do we still need that
> with this fix in place, or do we have the same issue elsewhere?
>
> On Mon, May 6, 2013 at 9:48 AM, Jordan Rose <jordan_rose at apple.com> wrote:
>
>> Author: jrose
>> Date: Mon May 6 11:48:12 2013
>> New Revision: 181212
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=181212&view=rev
>> Log:
>> Fix representation of compound literals for C++ objects with destructors.
>>
>> Previously, this compound literal expression (a GNU extension in C++):
>>
>> (AggregateWithDtor){1, 2}
>>
>> resulted in this AST:
>>
>> `-CXXBindTemporaryExpr [...] 'struct Point' (CXXTemporary [...])
>> `-CompoundLiteralExpr [...] 'struct AggregateWithDtor'
>> `-CXXBindTemporaryExpr [...] 'struct AggregateWithDtor'
>> (CXXTemporary [...])
>> `-InitListExpr [...] 'struct AggregateWithDtor'
>> |-IntegerLiteral [...] 'int' 1
>> `-IntegerLiteral [...] 'int' 2
>>
>> Note the two CXXBindTemporaryExprs. The InitListExpr is really part of the
>> CompoundLiteralExpr, not an object in its own right. By introducing a new
>> entity initialization kind in Sema specifically for compound literals, we
>> avoid the treatment of the inner InitListExpr as a temporary.
>>
>> `-CXXBindTemporaryExpr [...] 'struct Point' (CXXTemporary [...])
>> `-CompoundLiteralExpr [...] 'struct AggregateWithDtor'
>> `-InitListExpr [...] 'struct AggregateWithDtor'
>> |-IntegerLiteral [...] 'int' 1
>> `-IntegerLiteral [...] 'int' 2
>>
>> Modified:
>> cfe/trunk/include/clang/Sema/Initialization.h
>> cfe/trunk/lib/Sema/SemaExpr.cpp
>> cfe/trunk/lib/Sema/SemaInit.cpp
>> cfe/trunk/test/SemaCXX/compound-literal.cpp
>>
>> Modified: cfe/trunk/include/clang/Sema/Initialization.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Initialization.h?rev=181212&r1=181211&r2=181212&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Sema/Initialization.h (original)
>> +++ cfe/trunk/include/clang/Sema/Initialization.h Mon May 6 11:48:12 2013
>> @@ -75,7 +75,10 @@ public:
>> EK_ComplexElement,
>> /// \brief The entity being initialized is the field that captures a
>> /// variable in a lambda.
>> - EK_LambdaCapture
>> + EK_LambdaCapture,
>> + /// \brief The entity being initialized is the initializer for a
>> compound
>> + /// literal.
>> + EK_CompoundLiteralInit
>> };
>>
>> private:
>> @@ -118,8 +121,8 @@ private:
>> /// low bit indicating whether the parameter is "consumed".
>> uintptr_t Parameter;
>>
>> - /// \brief When Kind == EK_Temporary, the type source information for
>> - /// the temporary.
>> + /// \brief When Kind == EK_Temporary or EK_CompoundLiteralInit, the
>> type
>> + /// source information for the temporary.
>> TypeSourceInfo *TypeInfo;
>>
>> struct LN LocAndNRVO;
>> @@ -287,7 +290,16 @@ public:
>> SourceLocation Loc) {
>> return InitializedEntity(Var, Field, Loc);
>> }
>> -
>> +
>> + /// \brief Create the entity for a compound literal initializer.
>> + static InitializedEntity InitializeCompoundLiteralInit(TypeSourceInfo
>> *TSI) {
>> + InitializedEntity Result(EK_CompoundLiteralInit, SourceLocation(),
>> + TSI->getType());
>> + Result.TypeInfo = TSI;
>> + return Result;
>> + }
>> +
>> +
>> /// \brief Determine the kind of initialization.
>> EntityKind getKind() const { return Kind; }
>>
>> @@ -302,7 +314,7 @@ public:
>> /// \brief Retrieve complete type-source information for the object
>> being
>> /// constructed, if known.
>> TypeSourceInfo *getTypeSourceInfo() const {
>> - if (Kind == EK_Temporary)
>> + if (Kind == EK_Temporary || Kind == EK_CompoundLiteralInit)
>> return TypeInfo;
>>
>> return 0;
>>
>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=181212&r1=181211&r2=181212&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon May 6 11:48:12 2013
>> @@ -4509,7 +4509,7 @@ Sema::BuildCompoundLiteralExpr(SourceLoc
>> return ExprError();
>>
>> InitializedEntity Entity
>> - = InitializedEntity::InitializeTemporary(literalType);
>> + = InitializedEntity::InitializeCompoundLiteralInit(TInfo);
>> InitializationKind Kind
>> = InitializationKind::CreateCStyleCast(LParenLoc,
>> SourceRange(LParenLoc,
>> RParenLoc),
>>
>> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=181212&r1=181211&r2=181212&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaInit.cpp Mon May 6 11:48:12 2013
>> @@ -2394,6 +2394,7 @@ DeclarationName InitializedEntity::getNa
>> case EK_VectorElement:
>> case EK_ComplexElement:
>> case EK_BlockElement:
>> + case EK_CompoundLiteralInit:
>> return DeclarationName();
>> }
>>
>> @@ -2420,6 +2421,7 @@ DeclaratorDecl *InitializedEntity::getDe
>> case EK_ComplexElement:
>> case EK_BlockElement:
>> case EK_LambdaCapture:
>> + case EK_CompoundLiteralInit:
>> return 0;
>> }
>>
>> @@ -2437,6 +2439,7 @@ bool InitializedEntity::allowsNRVO() con
>> case EK_Member:
>> case EK_New:
>> case EK_Temporary:
>> + case EK_CompoundLiteralInit:
>> case EK_Base:
>> case EK_Delegating:
>> case EK_ArrayElement:
>> @@ -4468,6 +4471,7 @@ getAssignmentAction(const InitializedEnt
>> case InitializedEntity::EK_ComplexElement:
>> case InitializedEntity::EK_BlockElement:
>> case InitializedEntity::EK_LambdaCapture:
>> + case InitializedEntity::EK_CompoundLiteralInit:
>> return Sema::AA_Initializing;
>> }
>>
>> @@ -4490,6 +4494,7 @@ static bool shouldBindAsTemporary(const
>> case InitializedEntity::EK_Exception:
>> case InitializedEntity::EK_BlockElement:
>> case InitializedEntity::EK_LambdaCapture:
>> + case InitializedEntity::EK_CompoundLiteralInit:
>> return false;
>>
>> case InitializedEntity::EK_Parameter:
>> @@ -4520,6 +4525,7 @@ static bool shouldDestroyTemporary(const
>> case InitializedEntity::EK_Temporary:
>> case InitializedEntity::EK_ArrayElement:
>> case InitializedEntity::EK_Exception:
>> + case InitializedEntity::EK_CompoundLiteralInit:
>> return true;
>> }
>>
>> @@ -4601,6 +4607,7 @@ static SourceLocation getInitializationL
>> case InitializedEntity::EK_VectorElement:
>> case InitializedEntity::EK_ComplexElement:
>> case InitializedEntity::EK_BlockElement:
>> + case InitializedEntity::EK_CompoundLiteralInit:
>> return Initializer->getLocStart();
>> }
>> llvm_unreachable("missed an InitializedEntity kind?");
>> @@ -4828,6 +4835,31 @@ static bool isReferenceBinding(const Ini
>> s.Kind == InitializationSequence::SK_BindReferenceToTemporary;
>> }
>>
>> +/// Returns true if the parameters describe a constructor initialization
>> of
>> +/// an explicit temporary object, e.g. "Point(x, y)".
>> +static bool isExplicitTemporary(const InitializedEntity &Entity,
>> + const InitializationKind &Kind,
>> + unsigned NumArgs) {
>> + switch (Entity.getKind()) {
>> + case InitializedEntity::EK_Temporary:
>> + case InitializedEntity::EK_CompoundLiteralInit:
>> + break;
>> + default:
>> + return false;
>> + }
>> +
>> + switch (Kind.getKind()) {
>> + case InitializationKind::IK_DirectList:
>> + return true;
>> + // FIXME: Hack to work around cast weirdness.
>> + case InitializationKind::IK_Direct:
>> + case InitializationKind::IK_Value:
>> + return NumArgs != 1;
>> + default:
>> + return false;
>> + }
>> +}
>> +
>> static ExprResult
>> PerformConstructorInitialization(Sema &S,
>> const InitializedEntity &Entity,
>> @@ -4878,11 +4910,7 @@ PerformConstructorInitialization(Sema &S
>> return ExprError();
>>
>>
>> - if (Entity.getKind() == InitializedEntity::EK_Temporary &&
>> - (Kind.getKind() == InitializationKind::IK_DirectList ||
>> - (NumArgs != 1 && // FIXME: Hack to work around cast weirdness
>> - (Kind.getKind() == InitializationKind::IK_Direct ||
>> - Kind.getKind() == InitializationKind::IK_Value)))) {
>> + if (isExplicitTemporary(Entity, Kind, NumArgs)) {
>> // An explicitly-constructed temporary, e.g., X(1, 2).
>> S.MarkFunctionReferenced(Loc, Constructor);
>> if (S.DiagnoseUseOfDecl(Constructor, Loc))
>> @@ -4984,6 +5012,7 @@ InitializedEntityOutlivesFullExpression(
>> case InitializedEntity::EK_Parameter:
>> case InitializedEntity::EK_Temporary:
>> case InitializedEntity::EK_LambdaCapture:
>> + case InitializedEntity::EK_CompoundLiteralInit:
>> // The entity being initialized might not outlive the
>> full-expression.
>> return false;
>> }
>>
>> Modified: cfe/trunk/test/SemaCXX/compound-literal.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/compound-literal.cpp?rev=181212&r1=181211&r2=181212&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/compound-literal.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/compound-literal.cpp Mon May 6 11:48:12 2013
>> @@ -1,4 +1,8 @@
>> -// RUN: %clang_cc1 -fsyntax-only -verify %s
>> +// RUN: %clang_cc1 -fsyntax-only -std=c++03 -verify -ast-dump %s > %t-03
>> +// RUN: FileCheck --input-file=%t-03 %s
>> +// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify -ast-dump %s > %t-11
>> +// RUN: FileCheck --input-file=%t-11 %s
>> +// RUN: FileCheck --input-file=%t-11 %s --check-prefix=CHECK-CXX11
>>
>> // http://llvm.org/PR7905
>> namespace PR7905 {
>> @@ -12,3 +16,63 @@ void foo2() {
>> (void)(M<short> []) {{3}};
>> }
>> }
>> +
>> +// Check compound literals mixed with C++11 list-initialization.
>> +namespace brace_initializers {
>> + struct POD {
>> + int x, y;
>> + };
>> + struct HasCtor {
>> + HasCtor(int x, int y);
>> + };
>> + struct HasDtor {
>> + int x, y;
>> + ~HasDtor();
>> + };
>> + struct HasCtorDtor {
>> + HasCtorDtor(int x, int y);
>> + ~HasCtorDtor();
>> + };
>> +
>> + void test() {
>> + (void)(POD){1, 2};
>> + // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'struct
>> brace_initializers::POD'
>> + // CHECK: CompoundLiteralExpr {{.*}} 'struct brace_initializers::POD'
>> + // CHECK-NEXT: InitListExpr {{.*}} 'struct brace_initializers::POD'
>> + // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
>> + // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
>> +
>> + (void)(HasDtor){1, 2};
>> + // CHECK: CXXBindTemporaryExpr {{.*}} 'struct
>> brace_initializers::HasDtor'
>> + // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'struct
>> brace_initializers::HasDtor'
>> + // CHECK-NEXT: InitListExpr {{.*}} 'struct
>> brace_initializers::HasDtor'
>> + // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
>> + // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
>> +
>> +#if __cplusplus >= 201103L
>> + (void)(HasCtor){1, 2};
>> + // CHECK-CXX11-NOT: CXXBindTemporaryExpr {{.*}} 'struct
>> brace_initializers::HasCtor'
>> + // CHECK-CXX11: CompoundLiteralExpr {{.*}} 'struct
>> brace_initializers::HasCtor'
>> + // CHECK-CXX11-NEXT: CXXTemporaryObjectExpr {{.*}} 'struct
>> brace_initializers::HasCtor'
>> + // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 1{{$}}
>> + // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 2{{$}}
>> +
>> + (void)(HasCtorDtor){1, 2};
>> + // CHECK-CXX11: CXXBindTemporaryExpr {{.*}} 'struct
>> brace_initializers::HasCtorDtor'
>> + // CHECK-CXX11-NEXT: CompoundLiteralExpr {{.*}} 'struct
>> brace_initializers::HasCtorDtor'
>> + // CHECK-CXX11-NEXT: CXXTemporaryObjectExpr {{.*}} 'struct
>> brace_initializers::HasCtorDtor'
>> + // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 1{{$}}
>> + // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 2{{$}}
>> +#endif
>> + }
>> +
>> + struct PrivateDtor {
>> + int x, y;
>> + private:
>> + ~PrivateDtor(); // expected-note {{declared private here}}
>> + };
>> +
>> + void testPrivateDtor() {
>> + (void)(PrivateDtor){1, 2}; // expected-error {{temporary of type
>> 'brace_initializers::PrivateDtor' has private destructor}}
>> + }
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130506/dc257b6d/attachment.html>
More information about the cfe-commits
mailing list