r324062 - [Sema] Add implicit members even for invalid CXXRecordDecls

[Ilya Biryukov via cfe-commits]

I'll try to come up with a fix, thanks for spotting that.


On Sat, Feb 3, 2018 at 2:24 AM Eric Fiselier <eric at efcs.ca> wrote:

> This causes some stray diagnostics to be emitted in certian cases where a
> base class is already invalid:
>
> Reproducer:
> https://gist.github.com/EricWF/588a361030edeaebbbc1155b8347cab0
>
> On Fri, Feb 2, 2018 at 1:40 AM, Ilya Biryukov via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: ibiryukov
>> Date: Fri Feb  2 00:40:08 2018
>> New Revision: 324062
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=324062&view=rev
>> Log:
>> [Sema] Add implicit members even for invalid CXXRecordDecls
>>
>> Summary:
>> It should be safe, since other code paths are already generating
>> implicit members even in invalid CXXRecordDecls (e.g. lookup).
>>
>> If we don't generate implicit members on CXXRecordDecl's completion,
>> they will be generated by next lookup of constructors. This causes a
>> crash when the following conditions are met:
>>   - a CXXRecordDecl is invalid,
>>   - it is provided via ExternalASTSource (e.g. from PCH),
>>   - it has inherited constructors (they create ShadowDecls),
>>   - lookup of its constructors was not run before ASTWriter serialized
>>     it.
>>
>> This may require the ShadowDecls created for inherited constructors to
>> be removed from the class, but that's no longer possible since class is
>> provided by ExternalASTSource.
>>
>> See provided lit test for an example.
>>
>> Reviewers: bkramer
>>
>> Reviewed By: bkramer
>>
>> Subscribers: cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D42810
>>
>> Added:
>>     cfe/trunk/test/Index/Inputs/crash-preamble-classes.h
>>     cfe/trunk/test/Index/crash-preamble-classes.cpp
>> Modified:
>>     cfe/trunk/lib/Sema/SemaDecl.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=324062&r1=324061&r2=324062&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Feb  2 00:40:08 2018
>> @@ -15440,10 +15440,10 @@ void Sema::ActOnFields(Scope *S, SourceL
>>                                            CXXRecord->getDestructor());
>>          }
>>
>> -        if (!CXXRecord->isInvalidDecl()) {
>> -          // Add any implicitly-declared members to this class.
>> -          AddImplicitlyDeclaredMembersToClass(CXXRecord);
>> +        // Add any implicitly-declared members to this class.
>> +        AddImplicitlyDeclaredMembersToClass(CXXRecord);
>>
>> +        if (!CXXRecord->isInvalidDecl()) {
>>            // If we have virtual base classes, we may end up finding
>> multiple
>>            // final overriders for a given virtual function. Check for
>> this
>>            // problem now.
>>
>> Added: cfe/trunk/test/Index/Inputs/crash-preamble-classes.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Inputs/crash-preamble-classes.h?rev=324062&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Index/Inputs/crash-preamble-classes.h (added)
>> +++ cfe/trunk/test/Index/Inputs/crash-preamble-classes.h Fri Feb  2
>> 00:40:08 2018
>> @@ -0,0 +1,9 @@
>> +struct Incomplete;
>> +
>> +struct X : Incomplete {
>> +  X();
>> +};
>> +
>> +struct Y : X {
>> +  using X::X;
>> +};
>>
>> Added: cfe/trunk/test/Index/crash-preamble-classes.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/crash-preamble-classes.cpp?rev=324062&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Index/crash-preamble-classes.cpp (added)
>> +++ cfe/trunk/test/Index/crash-preamble-classes.cpp Fri Feb  2 00:40:08
>> 2018
>> @@ -0,0 +1,8 @@
>> +#include "crash-preamble-classes.h"
>> +
>> +struct Z : Y {
>> +  Z() {}
>> +};
>> +
>> +// RUN: env CINDEXTEST_EDITING=1 \
>> +// RUN: c-index-test -test-load-source-reparse 5 local -I %S/Inputs %s
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>

-- 
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180205/1dea2455/attachment-0001.html>