> On Sep 16, 2014, at 5:08 PM, John McCall <[email protected]> wrote:
>
> On Sep 16, 2014, at 5:06 PM, Ben Langmuir <[email protected]
> <mailto:[email protected]>> wrote:
>>> On Sep 16, 2014, at 1:25 PM, John McCall <[email protected]
>>> <mailto:[email protected]>> wrote:
>>>
>>> On Sep 16, 2014, at 12:54 PM, Richard Smith <[email protected]
>>> <mailto:[email protected]>> wrote:
>>>> On Tue, Sep 16, 2014 at 12:47 PM, John McCall <[email protected]
>>>> <mailto:[email protected]>> wrote:
>>>> On Sep 16, 2014, at 12:11 PM, Richard Smith <[email protected]
>>>> <mailto:[email protected]>> wrote:
>>>>> On Tue, Sep 16, 2014 at 11:45 AM, John McCall <[email protected]
>>>>> <mailto:[email protected]>> wrote:
>>>>> On Sep 15, 2014, at 9:47 AM, Ben Langmuir <[email protected]
>>>>> <mailto:[email protected]>> wrote:
>>>>> > Hi John,
>>>>> >
>>>>> > This patch fixes the assertion failure I talked to you about in
>>>>> > Objective C++ codegen. It turned out to have nothing to do with
>>>>> > templates.
>>>>> >
>>>>> > Fix an assertion failure trying to emit a trivial destructor in
>>>>> > ObjC++
>>>>> >
>>>>> > If a base class declares a destructor, we will add the implicit
>>>>> > destructor for the subclass in
>>>>> > ActOnFields -> AddImplicitlyDeclaredMembersToClass
>>>>> >
>>>>> > But in Objective C++, we did not compute whether we have a trivial
>>>>> > destructor until after that in
>>>>> > CXXRecordDecl::completeDefinition()
>>>>> >
>>>>> > This was leading to a mismatch between the class, which thought it
>>>>> > had
>>>>> > no trivial destructor, and the CXXDestructorDecl, which considered
>>>>> > itself trivial.
>>>>>
>>>>> I feel like hasTrivialDestructor should return the right value here. I
>>>>> understand (and am saddened by) the hack about not setting PlainOldData
>>>>> until completeDefinition, but maybe we can set/clear the rest of the bits
>>>>> eagerly?
>>>>>
>>>>> Why do we have to delay setting the PlainOldData flag?
>>>>
>>>> There is a diagnostic which wants to warn about structs that are only POD
>>>> in non-ARC modes.
>>>>
>>>> Thanks, I suspected something along those lines. Perhaps we could track
>>>> both properties and still perform the calculation eagerly:
>>>>
>>>> - bool isPOD() const { return data().PlainOldData; }
>>>> + bool isPOD() const { return data().PlainOldData &&
>>>> !data().HasARCObjectMember; }
>>>> + bool wouldHaveBeenPODIfItWerentForYouMeddlingKids() const { return
>>>> data().PlainOldData; }
>>>
>>> That works for me, or we could even give it its own bit in the definition
>>> data; it’s not like we aren’t tracking a number of other things there for
>>> similar purposes.
>>>
>>> John.
>>
>> John and I took a look and it turns out we killed the warning in question as
>> part of removing -Warc-abi. I’ve attached an updated patch that just
>> eagerly sets these bits in addedMember so we will get the correct value
>> inside AddImplicitlyDeclaredMembersToClass.
>
> Looks great to me; thanks, Ben.
>
> John.
Thanks John.
Richard, did you have any other feedback, or shall I commit?
Ben
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits