r328680 - [ObjC] Make C++ triviality type traits available to non-trivial C
Akira Hatanaka via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 9 12:47:23 PDT 2018
> On Apr 5, 2018, at 1:25 PM, John McCall <rjmccall at apple.com> wrote:
>
>
>
>> On Apr 5, 2018, at 3:54 PM, Akira Hatanaka <ahatanaka at apple.com <mailto:ahatanaka at apple.com>> wrote:
>>
>>
>>> On Apr 5, 2018, at 12:39 PM, John McCall <rjmccall at apple.com <mailto:rjmccall at apple.com>> wrote:
>>>
>>>
>>>
>>>> On Apr 4, 2018, at 7:37 PM, Akira Hatanaka <ahatanaka at apple.com <mailto:ahatanaka at apple.com>> wrote:
>>>>
>>>>
>>>>
>>>>> On Apr 4, 2018, at 4:24 PM, Akira Hatanaka via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>>>>>
>>>>>>
>>>>>> On Apr 4, 2018, at 3:38 PM, Richard Smith <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>> wrote:
>>>>>>
>>>>>> Hi Akira,
>>>>>>
>>>>>> This change has broken the C++ versions of these type traits for classes with volatile members. Such classes are required to claim to be trivial per C++ DR 2094 (http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2094 <http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2094>) but return false from isNonTrivialToPrimitiveCopy().
>>>>>>
>>>>>
>>>>> Oh that’s right. The function returns false when isNonTrivialToPrimitiveCopy() returns PCK_VolatileTrivial. That is wrong.
>>>>>
>>>>>> Also, exposing these __has_* traits more widely seems like a backwards step to me: these traits are deprecated, near-useless, and we're trying to remove them. No code should be using them under any circumstances; the __is_* traits should be used instead.
>>>>>>
>>>>>
>>>>> The __is_* traits (is_trivially_copy_constructible, etc.) are templates defined in libcxx, so it seems that we can’t use them when compiling in C mode. Is it OK to add their definitions to TokenKinds.def as non-C++ keywords?
>>>>>
>>>>
>>>> Or perhaps redefine the six __has_* traits used here as non-C++ (KEYNOCXX or ‘KEYC99 | KEYC11') keywords?
>>>
>>> I think Richard is talking about e.g. the __is_trivially_destructible intrinsic type trait function rather than std::is_trivially_destructible.
>>>
>>> Do we have a concrete need to expose these type traits to C?
>>>
>>
>> No, no one has asked any of these type traits to be exposed to C yet. Do you think we should just revert the patch and wait until someone asks for those type traits to be available in C?
>
> I think that would be fine. Although it would be nice if we could save the part of the patch that makes the trait logic sensitive to the type queries instead of needing to include duplicated checks for __strong, __weak, and whatever other non-trivial C features we eventually add.
>
Reverted r329289 in r329608. I wasn’t sure which part of the patch should be saved. I don’t think we need to check the properties of the types calling the QualType::isNonTrivialTo* functions since we aren’t exposing the type traits to C anymore?
> John.
>
>>
>>> John.
>>>
>>>>
>>>>>> On 27 March 2018 at 17:12, Akira Hatanaka via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>>>>>> Author: ahatanak
>>>>>> Date: Tue Mar 27 17:12:08 2018
>>>>>> New Revision: 328680
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=328680&view=rev <http://llvm.org/viewvc/llvm-project?rev=328680&view=rev>
>>>>>> Log:
>>>>>> [ObjC] Make C++ triviality type traits available to non-trivial C
>>>>>> structs.
>>>>>>
>>>>>> r326307 and r327870 made changes that allowed using non-trivial C
>>>>>> structs with fields qualified with __strong or __weak. This commit makes
>>>>>> the following C++ triviality type traits available to non-trivial C
>>>>>> structs:
>>>>>>
>>>>>> __has_trivial_assign
>>>>>> __has_trivial_move_assign
>>>>>> __has_trivial_copy
>>>>>> __has_trivial_move_constructor
>>>>>> __has_trivial_constructor
>>>>>> __has_trivial_destructor
>>>>>>
>>>>>> rdar://problem/33599681 <rdar://problem/33599681>
>>>>>>
>>>>>> Differential Revision: https://reviews.llvm.org/D44913 <https://reviews.llvm.org/D44913>
>>>>>>
>>>>>> Added:
>>>>>> cfe/trunk/test/SemaObjC/non-trivial-struct-traits.m
>>>>>> Modified:
>>>>>> cfe/trunk/include/clang/Basic/TokenKinds.def
>>>>>> cfe/trunk/lib/Sema/SemaExprCXX.cpp
>>>>>>
>>>>>> Modified: cfe/trunk/include/clang/Basic/TokenKinds.def
>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TokenKinds.def?rev=328680&r1=328679&r2=328680&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TokenKinds.def?rev=328680&r1=328679&r2=328680&view=diff>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/include/clang/Basic/TokenKinds.def (original)
>>>>>> +++ cfe/trunk/include/clang/Basic/TokenKinds.def Tue Mar 27 17:12:08 2018
>>>>>> @@ -433,12 +433,12 @@ TYPE_TRAIT_1(__has_nothrow_assign, HasNo
>>>>>> TYPE_TRAIT_1(__has_nothrow_move_assign, HasNothrowMoveAssign, KEYCXX)
>>>>>> TYPE_TRAIT_1(__has_nothrow_copy, HasNothrowCopy, KEYCXX)
>>>>>> TYPE_TRAIT_1(__has_nothrow_constructor, HasNothrowConstructor, KEYCXX)
>>>>>> -TYPE_TRAIT_1(__has_trivial_assign, HasTrivialAssign, KEYCXX)
>>>>>> -TYPE_TRAIT_1(__has_trivial_move_assign, HasTrivialMoveAssign, KEYCXX)
>>>>>> -TYPE_TRAIT_1(__has_trivial_copy, HasTrivialCopy, KEYCXX)
>>>>>> -TYPE_TRAIT_1(__has_trivial_constructor, HasTrivialDefaultConstructor, KEYCXX)
>>>>>> -TYPE_TRAIT_1(__has_trivial_move_constructor, HasTrivialMoveConstructor, KEYCXX)
>>>>>> -TYPE_TRAIT_1(__has_trivial_destructor, HasTrivialDestructor, KEYCXX)
>>>>>> +TYPE_TRAIT_1(__has_trivial_assign, HasTrivialAssign, KEYALL)
>>>>>> +TYPE_TRAIT_1(__has_trivial_move_assign, HasTrivialMoveAssign, KEYALL)
>>>>>> +TYPE_TRAIT_1(__has_trivial_copy, HasTrivialCopy, KEYALL)
>>>>>> +TYPE_TRAIT_1(__has_trivial_constructor, HasTrivialDefaultConstructor, KEYALL)
>>>>>> +TYPE_TRAIT_1(__has_trivial_move_constructor, HasTrivialMoveConstructor, KEYALL)
>>>>>> +TYPE_TRAIT_1(__has_trivial_destructor, HasTrivialDestructor, KEYALL)
>>>>>> TYPE_TRAIT_1(__has_virtual_destructor, HasVirtualDestructor, KEYCXX)
>>>>>> TYPE_TRAIT_1(__is_abstract, IsAbstract, KEYCXX)
>>>>>> TYPE_TRAIT_1(__is_aggregate, IsAggregate, KEYCXX)
>>>>>>
>>>>>> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=328680&r1=328679&r2=328680&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=328680&r1=328679&r2=328680&view=diff>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
>>>>>> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Mar 27 17:12:08 2018
>>>>>> @@ -4513,6 +4513,8 @@ static bool EvaluateUnaryTypeTrait(Sema
>>>>>> // does not correctly compute triviality in the presence of multiple special
>>>>>> // members of the same kind. Revisit this once the g++ bug is fixed.
>>>>>> case UTT_HasTrivialDefaultConstructor:
>>>>>> + if (T.isNonTrivialToPrimitiveDefaultInitialize())
>>>>>> + return false;
>>>>>> // http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html <http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html>:
>>>>>> // If __is_pod (type) is true then the trait is true, else if type is
>>>>>> // a cv class or union type (or array thereof) with a trivial default
>>>>>> @@ -4524,6 +4526,8 @@ static bool EvaluateUnaryTypeTrait(Sema
>>>>>> !RD->hasNonTrivialDefaultConstructor();
>>>>>> return false;
>>>>>> case UTT_HasTrivialMoveConstructor:
>>>>>> + if (T.isNonTrivialToPrimitiveDestructiveMove())
>>>>>> + return false;
>>>>>> // This trait is implemented by MSVC 2012 and needed to parse the
>>>>>> // standard library headers. Specifically this is used as the logic
>>>>>> // behind std::is_trivially_move_constructible (20.9.4.3).
>>>>>> @@ -4533,6 +4537,8 @@ static bool EvaluateUnaryTypeTrait(Sema
>>>>>> return RD->hasTrivialMoveConstructor() && !RD->hasNonTrivialMoveConstructor();
>>>>>> return false;
>>>>>> case UTT_HasTrivialCopy:
>>>>>> + if (T.isNonTrivialToPrimitiveCopy())
>>>>>> + return false;
>>>>>> // http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html <http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html>:
>>>>>> // If __is_pod (type) is true or type is a reference type then
>>>>>> // the trait is true, else if type is a cv class or union type
>>>>>> @@ -4545,6 +4551,8 @@ static bool EvaluateUnaryTypeTrait(Sema
>>>>>> !RD->hasNonTrivialCopyConstructor();
>>>>>> return false;
>>>>>> case UTT_HasTrivialMoveAssign:
>>>>>> + if (T.isNonTrivialToPrimitiveDestructiveMove())
>>>>>> + return false;
>>>>>> // This trait is implemented by MSVC 2012 and needed to parse the
>>>>>> // standard library headers. Specifically it is used as the logic
>>>>>> // behind std::is_trivially_move_assignable (20.9.4.3)
>>>>>> @@ -4554,6 +4562,8 @@ static bool EvaluateUnaryTypeTrait(Sema
>>>>>> return RD->hasTrivialMoveAssignment() && !RD->hasNonTrivialMoveAssignment();
>>>>>> return false;
>>>>>> case UTT_HasTrivialAssign:
>>>>>> + if (T.isNonTrivialToPrimitiveCopy())
>>>>>> + return false;
>>>>>> // http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html <http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html>:
>>>>>> // If type is const qualified or is a reference type then the
>>>>>> // trait is false. Otherwise if __is_pod (type) is true then the
>>>>>> @@ -4624,6 +4634,8 @@ static bool EvaluateUnaryTypeTrait(Sema
>>>>>> return true;
>>>>>>
>>>>>> case UTT_HasTrivialDestructor:
>>>>>> + if (T.isDestructedType() == QualType::DK_nontrivial_c_struct)
>>>>>> + return false;
>>>>>> // http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html <http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html>
>>>>>> // If __is_pod (type) is true or type is a reference type
>>>>>> // then the trait is true, else if type is a cv class or union
>>>>>>
>>>>>> Added: cfe/trunk/test/SemaObjC/non-trivial-struct-traits.m
>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/non-trivial-struct-traits.m?rev=328680&view=auto <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/non-trivial-struct-traits.m?rev=328680&view=auto>
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/test/SemaObjC/non-trivial-struct-traits.m (added)
>>>>>> +++ cfe/trunk/test/SemaObjC/non-trivial-struct-traits.m Tue Mar 27 17:12:08 2018
>>>>>> @@ -0,0 +1,25 @@
>>>>>> +// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -verify %s
>>>>>> +
>>>>>> +// expected-no-diagnostics
>>>>>> +
>>>>>> +struct Trivial {
>>>>>> + int x;
>>>>>> +};
>>>>>> +
>>>>>> +struct NonTrivial {
>>>>>> + id x;
>>>>>> +};
>>>>>> +
>>>>>> +int trivial_assign[__has_trivial_assign(struct Trivial) ? 1 : -1];
>>>>>> +int trivial_move_assign[__has_trivial_move_assign(struct Trivial) ? 1 : -1];
>>>>>> +int trivial_copy_constructor[__has_trivial_copy(struct Trivial) ? 1 : -1];
>>>>>> +int trivial_move_constructor[__has_trivial_move_constructor(struct Trivial) ? 1 : -1];
>>>>>> +int trivial_constructor[__has_trivial_constructor(struct Trivial) ? 1 : -1];
>>>>>> +int trivial_destructor[__has_trivial_destructor(struct Trivial) ? 1 : -1];
>>>>>> +
>>>>>> +int non_trivial_assign[__has_trivial_assign(struct NonTrivial) ? -1 : 1];
>>>>>> +int non_trivial_move_assign[__has_trivial_move_assign(struct NonTrivial) ? -1 : 1];
>>>>>> +int non_trivial_copy_constructor[__has_trivial_copy(struct NonTrivial) ? -1 : 1];
>>>>>> +int non_trivial_move_constructor[__has_trivial_move_constructor(struct NonTrivial) ? -1 : 1];
>>>>>> +int non_trivial_constructor[__has_trivial_constructor(struct NonTrivial) ? -1 : 1];
>>>>>> +int non_trivial_destructor[__has_trivial_destructor(struct NonTrivial) ? -1 : 1];
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> cfe-commits mailing list
>>>>>> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> cfe-commits mailing list
>>>>> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180409/c177f707/attachment-0001.html>
More information about the cfe-commits
mailing list