> On Apr 5, 2018, at 3:54 PM, Akira Hatanaka <ahatan...@apple.com> wrote: > > >> On Apr 5, 2018, at 12:39 PM, John McCall <rjmcc...@apple.com >> <mailto:rjmcc...@apple.com>> wrote: >> >> >> >>> On Apr 4, 2018, at 7:37 PM, Akira Hatanaka <ahatan...@apple.com >>> <mailto:ahatan...@apple.com>> wrote: >>> >>> >>> >>>> On Apr 4, 2018, at 4:24 PM, Akira Hatanaka via cfe-commits >>>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >>>> >>>>> >>>>> On Apr 4, 2018, at 3:38 PM, Richard Smith <rich...@metafoo.co.uk >>>>> <mailto:rich...@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. John. > >> John. >> >>> >>>>> On 27 March 2018 at 17:12, Akira Hatanaka via cfe-commits >>>>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@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@lists.llvm.org <mailto:cfe-commits@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@lists.llvm.org <mailto:cfe-commits@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@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
