+Hans for managing the 8.0 branch. On 1/31/19, 4:15 PM, "ahatan...@apple.com on behalf of Akira Hatanaka" <ahatan...@apple.com> wrote:
Reverted patch in r352822. I’ll send a new patch later that disallows trivial_abi on classes without non-deleted copy or move constructors. > On Jan 31, 2019, at 3:52 PM, Richard Smith <rich...@metafoo.co.uk> wrote: > > Given that there's uncertainty as to how to proceed and this patch > affects the ABI, I would prefer that we revert it for trunk and 8.0. > > The suggested alternative of disallowing trivial_abi in the case where > all copy/move constructors are deleted seems reasonable to me. > > On Thu, 31 Jan 2019 at 14:31, Shoaib Meenai via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> >> Just wanted to point out that r350920 is on the 8.0 release branch as well. I don't know if there are any additional considerations there. >> >> On 1/31/19, 2:20 PM, "cfe-commits on behalf of John McCall via cfe-commits" <cfe-commits-boun...@lists.llvm.org on behalf of cfe-commits@lists.llvm.org> wrote: >> >> >> >> On 31 Jan 2019, at 16:57, Akira Hatanaka wrote: >> >>> Would it be better if we disallowed trivial_abi if the class’ copy >>> and move destructors were all deleted (and revert r350920)? I think >>> that would make it easier to reason about when you are allowed to use >>> trivial_abi and what effect the attribute has (which is to override >>> the trivialness for the purpose of calls). >>> >>> Sorry for my late reply. It took a while to understand that the patch >>> I committed might not be the right way to fix the problem. >> >> I'd be fine with that. If nothing else, we can generalize it later if >> we decide that's an important use-case. >> >> John. >> >>> >>>> On Jan 16, 2019, at 8:37 PM, John McCall via cfe-commits >>>> <cfe-commits@lists.llvm.org> wrote: >>>> >>>> On 16 Jan 2019, at 20:03, Richard Smith wrote: >>>> >>>>> On Wed, 16 Jan 2019 at 16:20, John McCall via cfe-commits < >>>>> cfe-commits@lists.llvm.org> wrote: >>>>> >>>>>> On 16 Jan 2019, at 18:32, Richard Smith wrote: >>>>>> >>>>>>> On Wed, 16 Jan 2019 at 09:10, John McCall via cfe-commits < >>>>>>> cfe-commits@lists.llvm.org> wrote: >>>>>>> >>>>>>>> On 16 Jan 2019, at 9:13, Aaron Ballman wrote: >>>>>>>> >>>>>>>>> On Wed, Jan 16, 2019 at 1:57 AM Akira Hatanaka >>>>>>>>> <ahatan...@apple.com> >>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>> Yes, the behavior of the compiler doesn’t match what’s >>>>>>>>>> explained >>>>>>>>>> in the documentation anymore. >>>>>>>>>> >>>>>>>>>> Please take a look at the attached patch, which updates the >>>>>>>>>> documentation. >>>>>>>>> >>>>>>>>> Patch mostly LGTM, but I did have one wording suggestion. >>>>>>>>> >>>>>>>>>> diff --git a/include/clang/Basic/AttrDocs.td >>>>>>>>>> b/include/clang/Basic/AttrDocs.td >>>>>>>>>> index 5773a92c9c..ca3cfcf9b2 100644 >>>>>>>>>> --- a/include/clang/Basic/AttrDocs.td >>>>>>>>>> +++ b/include/clang/Basic/AttrDocs.td >>>>>>>>>> @@ -2478,15 +2478,20 @@ def TrivialABIDocs : Documentation { >>>>>>>>>> let Category = DocCatVariable; >>>>>>>>>> let Content = [{ >>>>>>>>>> The ``trivial_abi`` attribute can be applied to a C++ class, >>>>>>>>>> struct, >>>>>>>>>> or union. >>>>>>>>>> -It instructs the compiler to pass and return the type using >>>>>>>>>> the C >>>>>>>>>> ABI for the >>>>>>>>>> +``trivial_abi`` has the following effects: >>>>>>>>>> + >>>>>>>>>> +- It instructs the compiler to pass and return the type using >>>>>>>>>> the C >>>>>>>>>> ABI for the >>>>>>>>>> underlying type when the type would otherwise be considered >>>>>>>>>> non-trivial for the >>>>>>>>>> purpose of calls. >>>>>>>>>> -A class annotated with `trivial_abi` can have non-trivial >>>>>>>>>> destructors or copy/move constructors without automatically >>>>>>>>>> becoming >>>>>>>>>> non-trivial for the purposes of calls. For example: >>>>>>>>>> +- It makes the destructor and copy and move constructors of >>>>>>>>>> the >>>>>>>>>> class trivial >>>>>>>>>> +that would otherwise be considered non-trivial under the C++ >>>>>>>>>> ABI >>>>>>>>>> rules. >>>>>>>>> >>>>>>>>> How about: It makes the destructor, copy constructors, and move >>>>>>>>> constructors of the class trivial even if they would otherwise >>>>>>>>> be >>>>>>>>> non-trivial under the C++ ABI rules. >>>>>>>> >>>>>>>> Let's not say that it makes them trivial, because it doesn't. It >>>>>>>> causes >>>>>>>> their immediate non-triviality to be ignored for the purposes of >>>>>>>> deciding >>>>>>>> whether the type can be passed in registers. >>>>>>>> >>>>>>> >>>>>>> Given the attribute now forces the type to be passed in registers, >>>>>>> I >>>>>> think >>>>>>> it'd be more to the point to say that it makes the triviality of >>>>>>> those >>>>>>> special members be ignored when deciding whether to pass a type >>>>>>> with a >>>>>>> subobject of this type in registers. >>>>>> >>>>>> Wait, it forces it to be passed in registers? I thought the design >>>>>> was that it didn't override the non-trivial-abi-ness of subobjects; >>>>>> see all the discussion of trivially_relocatable. >>>>>> >>>>> >>>>> The attribute is ill-formed if applied to a class that has a >>>>> subobject that >>>>> can't be passed in registers (or that has a vptr). And then as a >>>>> weird >>>>> special case we don't instantiate the attribute when instantiating a >>>>> class >>>>> if it would be ill-formed (well, we instantiate it and then remove >>>>> it >>>>> again, but the effect is the same). >>>>> >>>>> The commit at the start of this email chain switches us from the >>>>> "just >>>>> override the trivialness for the purposes of the ABI" model to >>>>> /also/ >>>>> forcing the type to be passed in registers (the type would otherwise >>>>> not be >>>>> passed in registers in some corner cases, such as if all its >>>>> copy/move >>>>> special members are deleted). >>>> >>>> I see. Alright, I accept your wording, then. >>>> >>>> John. >>>> _______________________________________________ >>>> cfe-commits mailing list >>>> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=ywn0RCUoTpZm8YOV2ffRUUVgMx3xapVXDF-yihR7ycI&s=P5RqazYFOIlJWDGViplbmVcGCnxco2SFRE8jbjEiVIY&e= >>>> <https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=ywn0RCUoTpZm8YOV2ffRUUVgMx3xapVXDF-yihR7ycI&s=P5RqazYFOIlJWDGViplbmVcGCnxco2SFRE8jbjEiVIY&e=> >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=ywn0RCUoTpZm8YOV2ffRUUVgMx3xapVXDF-yihR7ycI&s=RqlQW1jluoVzRIWOqcAMcLk2-6YMFlflJblR_OplhVw&e= >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwIFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=g5EPqCUh5saO6axHZfXGpX8kSZZLUbixkcCIR9Lcc9o&s=NhJSDmoJK2gLjExO78nxiJS_w5UwzGvCIuEO4zaUVRI&e= _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits