__is_constructible return true for abstract types PR19178

[Richard Smith]

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/20140414/3f5ae261/attachment.html>