[cfe-commits] [PATCH] PR13788 - sema crash on add initializer

[Brian Brooks]

Ping!

On Fri, Nov 2, 2012 at 11:52 PM, Brian Brooks <brooks.brian at gmail.com>wrote:

> >> I'm not sure I follow how this comment relates to the patch.
>
> I see that a CXXRecordDecl is created during template instantiation via
> RequireCompleteType().  The abort is triggered by dyn_cast to a
> CXXRecordDecl before it has been created.  I am not sure if the Right thing
> to do is instantiate the template here, thus creating the CXXRecordDecl,
> instead of just returning if the declarator is dependently typed.  Sorry if
> this doubt doesn't make sense.. I am still very new to the code base and
> the world of compiling.
>
> I have attached an updated patch.  Thanks for the feedback!
>
> Brian
>
>
> On Thu, Nov 1, 2012 at 6:52 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>
>> On Thu, Nov 1, 2012 at 5:36 AM, Brian Brooks <brooks.brian at gmail.com>
>> wrote:
>> > Adding cfe-dev...
>>
>> Dropping cfe-dev via bcc, you were right first time.
>>
>> > On Wed, Oct 31, 2012 at 5:26 PM, Brian Brooks <brooks.brian at gmail.com>
>> > wrote:
>> >> The attached patch needs review please!
>> >>
>> >> I suspect that we may need to call RequireCompleteType() to do
>> >> instantiation *even if* the VDecl is a dependent type.  I am not yet
>> able to
>> >> reason about this though.
>>
>> I'm not sure I follow how this comment relates to the patch.
>>
>> >> The patch prevents the abort, and the test case (extended with code to
>> >> print initialized data) output looks the same in clang and gcc.
>>
>> -  QualType baseType = Context.getBaseElementType(var->getType());
>> -  if (baseType->isDependentType()) return;
>> +  QualType type = var->getType();
>> +  QualType baseType = Context.getBaseElementType(type);
>> +  if (type->isDependentType() || baseType->isDependentType())
>> +    return;
>>
>> You only need to check 'type' here. It will be dependent whenever
>> 'baseType' is.
>>
>> --- test/SemaCXX/PR13788.cpp    (revision 0)
>> +++ test/SemaCXX/PR13788.cpp    (revision 0)
>>
>> We prefer to add tests to existing test files when that is reasonable.
>> test/SemaTemplate/dependent-sized_array.cpp seems like a good place
>> for this test (please add a comment pointing at PR13788 to the test
>> case, or put it in namespace PR13788, or similar).
>>
>> Thanks!
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121108/67371508/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dependent-array-initializer.patch
Type: application/octet-stream
Size: 1735 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121108/67371508/attachment.obj>