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

[Brian Brooks]

>> 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/20121102/379bcd91/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/20121102/379bcd91/attachment.obj>