__is_constructible return true for abstract types PR19178

[Nikola Smiljanic]

I had a quick look at RequireNonAbstractType but as you said it only
provides diagnostic and completes the type which we already did.

I'll add tests for other two traits. I have commit access.

Thanks.


On Tue, Apr 15, 2014 at 10:44 AM, Richard Smith <richard at metafoo.co.uk>wrote:

> I'd like a little more test coverage (for the other two type traits that
> hit this codepath), but otherwise LGTM.
>
> It doesn't matter whether we call RequireNonAbstractType or just
> isAbstract() here, because we don't need to complete the type (we did that
> a few lines above) and we don't care about its ability to produce nice
> diagnostics. Using isAbstract() is more consistent with the
> isIncompleteType() call immediately above, so the code change seems fine as
> is.
>
> I forget, do you have commit access?
>
>
> On Mon, Apr 14, 2014 at 10:16 AM, Alp Toker <alp at nuanti.com> wrote:
>
>>
>> On 14/04/2014 11:59, Nikola Smiljanic wrote:
>>
>>> Please review.
>>>
>>> pr19178.patch
>>>
>>>
>>> diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
>>> index 8b9c0e2..2ff501f 100644
>>> --- a/lib/Sema/SemaExprCXX.cpp
>>> +++ b/lib/Sema/SemaExprCXX.cpp
>>> @@ -3656,6 +3656,12 @@ static bool evaluateTypeTrait(Sema &S, TypeTrait
>>> Kind, SourceLocation KWLoc,
>>>       if (Args[0]->getType()->isIncompleteType())
>>>         return false;
>>>   +    // Make sure the first argument is not an abstract type.
>>> +
>>> +    CXXRecordDecl *RD = Args[0]->getType()->getAsCXXRecordDecl();
>>> +    if (RD && RD->isAbstract())
>>> +      return false;
>>> +
>>>
>>
>> The fix is correct. Perhaps you could use RequireNonAbstractType()
>> instead of checking for CXXRecordDecl directly, something like this taken
>> from the counterpart in BTT_IsConvertible:
>>
>>
>> -    // Make sure the first argument is a complete type.
>> -    if (Args[0]->getType()->isIncompleteType())
>> +    // Make sure the first argument is a complete non-abstract type.
>> +    if (S.RequireCompleteType(KWLoc, Args[0]->getType(), 0) ||
>> +        S.RequireNonAbstractType(KWLoc, Args[0]->getType(), 0))
>>        return false;
>>
>> Either way looks good to me, thanks!
>>
>> Alp.
>>
>>
>>        SmallVector<OpaqueValueExpr, 2> OpaqueArgExprs;
>>>       SmallVector<Expr *, 2> ArgExprs;
>>>       ArgExprs.reserve(Args.size() - 1);
>>> diff --git a/test/SemaCXX/type-traits.cpp b/test/SemaCXX/type-traits.cpp
>>> index 28fb8dc..b8557c4 100644
>>> --- a/test/SemaCXX/type-traits.cpp
>>> +++ b/test/SemaCXX/type-traits.cpp
>>> @@ -1964,6 +1964,8 @@ void constructible_checks() {
>>>       { int arr[T(__is_constructible(NonPOD, int))]; }
>>>     { int arr[F(__is_nothrow_constructible(NonPOD, int))]; }
>>> +
>>> +  { int arr[F(__is_constructible(Abstract))]; } // PR19178
>>>   }
>>>     // Instantiation of __is_trivially_constructible
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>> --
>> http://www.nuanti.com
>> the browser experts
>>
>> _______________________________________________
>> 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/20140415/3f4d0bd5/attachment.html>