Thanks! I've merged r352822 to the branch in r352855.
On Fri, Feb 1, 2019 at 1:37 AM Shoaib Meenai <smee...@fb.com> wrote:
>
> +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
