Message ID | CAH6eHdQGtTokh6epA-6SthhtVzSZVEoUuFndzBjRt_jjy_i9_g@mail.gmail.com |
---|---|
State | New |
Headers | show |
On 12/11/2011 04:05 PM, Jonathan Wakely wrote:
> ping
In my opinion __is_final would be definitely useful in general, for 4.8,
and 4.7 too, if isn't too late.
As regards the wider issue which is being discussed on the reflector -
beware, I didn't follow all the messages - 'final' disabling a nice
optimization like EBO makes me very nervous. Really, doesn't seem part
of the intended general philosophy in this area. There must be a way to
overcome the annoyance. Last resort, if suggestions like having 'final'
not forbidding private derivation cannot go through, we could imagine a
GCC attribute reverting the effect of 'final' for people (library
writers ;) knowing what they are doing. I don't know.
Paolo.
On 12/12/2011, Paolo Carlini wrote: > On 12/11/2011 04:05 PM, Jonathan Wakely wrote: >> ping > In my opinion __is_final would be definitely useful in general, for 4.8, > and 4.7 too, if isn't too late. As we've got the final keyword in 4.7 I think we really want __is_final in the front end too. > As regards the wider issue which is being discussed on the reflector - > beware, I didn't follow all the messages - 'final' disabling a nice > optimization like EBO makes me very nervous. Really, doesn't seem part > of the intended general philosophy in this area. There must be a way to > overcome the annoyance. Last resort, if suggestions like having 'final' > not forbidding private derivation cannot go through, we could imagine a > GCC attribute reverting the effect of 'final' for people (library > writers ;) knowing what they are doing. I don't know. I think being able to detect a final class is good enough for now, until we find out if there are real problems being encountered as people make more use of C++11.
On 12/12/2011 12:19 PM, Jonathan Wakely wrote: >> As regards the wider issue which is being discussed on the reflector - >> beware, I didn't follow all the messages - 'final' disabling a nice >> optimization like EBO makes me very nervous. Really, doesn't seem part >> of the intended general philosophy in this area. There must be a way to >> overcome the annoyance. Last resort, if suggestions like having 'final' >> not forbidding private derivation cannot go through, we could imagine a >> GCC attribute reverting the effect of 'final' for people (library >> writers ;) knowing what they are doing. I don't know. > I think being able to detect a final class is good enough for now, > until we find out if there are real problems being encountered as > people make more use of C++11. Maybe. But in my opinion we should not rush. Something is wrong here at a more fundamental level. Paolo.
On Mon, Dec 12, 2011 at 5:25 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote: >> I think being able to detect a final class is good enough for now, >> until we find out if there are real problems being encountered as >> people make more use of C++11. > > Maybe. But in my opinion we should not rush. Something is wrong here at a > more fundamental level. > I agree that we should wait a little bit for the dust to settle down. Users should avoid it, and implementors shouldn't go through hoops non commensurable with the benefits of "final". Maybe the "right" primitive is slightly different.
On 12/12/2011 09:14 AM, Gabriel Dos Reis wrote: > On Mon, Dec 12, 2011 at 5:25 AM, Paolo Carlini<paolo.carlini@oracle.com> wrote: > >>> I think being able to detect a final class is good enough for now, >>> until we find out if there are real problems being encountered as >>> people make more use of C++11. >> >> Maybe. But in my opinion we should not rush. Something is wrong here at a >> more fundamental level. > > I agree that we should wait a little bit for the dust to settle down. Users should > avoid it, and implementors shouldn't go through hoops non commensurable with > the benefits of "final". Maybe the "right" primitive is slightly different. This patch seems pretty simple and safe. Are you (Gaby and Paolo) arguing that even so, it shouldn't go in? Jason
Hi, > > This patch seems pretty simple and safe. Are you (Gaby and Paolo) arguing that even so, it shouldn't go in? As far as I'm concerned, definetely not! I also think that it would be great if, for 4.7, Jon could handle the library issues with EBO by exploiting it. I only meant to say that something seems to me more fundamentally wrong at the design level about 'final' vs EBO, my hope is that for 4.8 we'll have a longer term stable solution based on a ISO Committee position. Paolo
On 13 December 2011 17:01, Paolo Carlini <pcarlini@gmail.com> wrote: > Hi, >> >> This patch seems pretty simple and safe. Are you (Gaby and Paolo) arguing that even so, it shouldn't go in? > > As far as I'm concerned, definetely not! I also think that it would be great if, for 4.7, Jon could handle the library issues with EBO by exploiting it. Yes, if this goes in for 4.7 I will definitely follow it with the library changes to make use of it. > I only meant to say that something seems to me more fundamentally wrong at the design level about 'final' vs EBO, my hope is that for 4.8 we'll have a longer term stable solution based on a ISO Committee position. In one of the earlier bug report comments I proposed a __gnu_cxx::is_final<T> library trait to expose the __is_final(T) intrinsic for users, but decided against it precisely because I don't know how the committee will want to deal with the issue longer term. So the proposed patch just adds the __is_final intrinsic for use internally by the library, to allow library changes so the test cases in the bug report will pass. If preferred I won't even add __is_final to the extend.texi docs, to leave it as an undocumented extension that we could more easily remove (or deprecate) later if necessary.
Hi, > So the proposed patch just adds the __is_final intrinsic for use > internally by the library, to allow library changes so the test cases > in the bug report will pass. If preferred I won't even add __is_final > to the extend.texi docs, to leave it as an undocumented extension that > we could more easily remove (or deprecate) later if necessary. I think this makes absolutely sense. And documenting it seems fine: after all, for better and worse, 'final' itself is in C++11. For the internal library uses, on the other hand, I don't think we should overly document a trait like __is_empty_non_final (essentially an __and_) which, afaics, would usefully replace std::is_empty in various places for 4.7. Paolo
On 12/13/2011 12:01 PM, Paolo Carlini wrote:
> As far as I'm concerned, definetely not! I also think that it would be great if, for 4.7, Jon could handle the library issues with EBO by exploiting it.
The patch is OK, then.
Jason
On Tue, Dec 13, 2011 at 10:43 AM, Jason Merrill <jason@redhat.com> wrote: > On 12/12/2011 09:14 AM, Gabriel Dos Reis wrote: >> >> On Mon, Dec 12, 2011 at 5:25 AM, Paolo Carlini<paolo.carlini@oracle.com> >> wrote: >> >>>> I think being able to detect a final class is good enough for now, >>>> until we find out if there are real problems being encountered as >>>> people make more use of C++11. >>> >>> >>> Maybe. But in my opinion we should not rush. Something is wrong here at a >>> more fundamental level. >> >> >> I agree that we should wait a little bit for the dust to settle down. >> Users should >> avoid it, and implementors shouldn't go through hoops non commensurable >> with >> the benefits of "final". Maybe the "right" primitive is slightly >> different. > > > This patch seems pretty simple and safe. Are you (Gaby and Paolo) arguing > that even so, it shouldn't go in? My comment isn't about the technical aspect of the patch itself -- it is a simple patch. But we don't seem to understand yet the implications of "final". As was observed on the standard reflectors, the appropriate trait might actually need to be binary instead of unary as this patch implements -- for the purpose of empty base optimization which is where the problems pop up.
On Tue, Dec 13, 2011 at 12:02 PM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > On 13 December 2011 17:01, Paolo Carlini <pcarlini@gmail.com> wrote: >> Hi, >>> >>> This patch seems pretty simple and safe. Are you (Gaby and Paolo) arguing that even so, it shouldn't go in? >> >> As far as I'm concerned, definetely not! I also think that it would be great if, for 4.7, Jon could handle the library issues with EBO by exploiting it. > > Yes, if this goes in for 4.7 I will definitely follow it with the > library changes to make use of it. > >> I only meant to say that something seems to me more fundamentally wrong at the design level about 'final' vs EBO, my hope is that for 4.8 we'll have a longer term stable solution based on a ISO Committee position. > > In one of the earlier bug report comments I proposed a > __gnu_cxx::is_final<T> library trait to expose the __is_final(T) > intrinsic for users, but decided against it precisely because I don't > know how the committee will want to deal with the issue longer term. > > So the proposed patch just adds the __is_final intrinsic for use > internally by the library, to allow library changes so the test cases > in the bug report will pass. If preferred I won't even add __is_final > to the extend.texi docs, to leave it as an undocumented extension that > we could more easily remove (or deprecate) later if necessary. Leaving __is_final undocumented is probably a good compromise.
Hi, > My comment isn't about the technical aspect of the patch itself -- it > is a simple patch. But we don't seem to understand yet the > implications of "final". As was observed on the standard reflectors, > the appropriate trait might actually need to be binary instead of > unary as this patch implements -- for the purpose of empty base > optimization which is where the problems pop up. Interesting the binary vs unary thing, makes me curious to re-read the whole thread. Then indeed, makes sense to be safe and not documenting for 4.7 the unary version which we are going to use right now for the immediate needs of the library. Paolo.
Index: c-family/c-common.c =================================================================== --- c-family/c-common.c (revision 181967) +++ c-family/c-common.c (working copy) @@ -462,6 +462,7 @@ const struct c_common_resword c_common_r { "__is_convertible_to", RID_IS_CONVERTIBLE_TO, D_CXXONLY }, { "__is_empty", RID_IS_EMPTY, D_CXXONLY }, { "__is_enum", RID_IS_ENUM, D_CXXONLY }, + { "__is_final", RID_IS_FINAL, D_CXXONLY }, { "__is_literal_type", RID_IS_LITERAL_TYPE, D_CXXONLY }, { "__is_pod", RID_IS_POD, D_CXXONLY }, { "__is_polymorphic", RID_IS_POLYMORPHIC, D_CXXONLY }, Index: c-family/c-common.h =================================================================== --- c-family/c-common.h (revision 181967) +++ c-family/c-common.h (working copy) @@ -134,7 +134,7 @@ enum rid RID_CONSTCAST, RID_DYNCAST, RID_REINTCAST, RID_STATCAST, /* C++ extensions */ - RID_BASES, RID_DIRECT_BASES, + RID_BASES, RID_DIRECT_BASES, RID_HAS_NOTHROW_ASSIGN, RID_HAS_NOTHROW_CONSTRUCTOR, RID_HAS_NOTHROW_COPY, RID_HAS_TRIVIAL_ASSIGN, RID_HAS_TRIVIAL_CONSTRUCTOR, RID_HAS_TRIVIAL_COPY, @@ -142,12 +142,12 @@ enum rid RID_IS_ABSTRACT, RID_IS_BASE_OF, RID_IS_CLASS, RID_IS_CONVERTIBLE_TO, RID_IS_EMPTY, RID_IS_ENUM, - RID_IS_LITERAL_TYPE, RID_IS_POD, - RID_IS_POLYMORPHIC, RID_IS_STD_LAYOUT, - RID_IS_TRIVIAL, RID_IS_UNION, - RID_UNDERLYING_TYPE, + RID_IS_FINAL, RID_IS_LITERAL_TYPE, + RID_IS_POD, RID_IS_POLYMORPHIC, + RID_IS_STD_LAYOUT, RID_IS_TRIVIAL, + RID_IS_UNION, RID_UNDERLYING_TYPE, - /* C++0x */ + /* C++11 */ RID_CONSTEXPR, RID_DECLTYPE, RID_NOEXCEPT, RID_NULLPTR, RID_STATIC_ASSERT, /* Objective-C ("AT" reserved words - they are only keywords when Index: cp/cp-tree.h =================================================================== --- cp/cp-tree.h (revision 181967) +++ cp/cp-tree.h (working copy) @@ -584,6 +584,7 @@ typedef enum cp_trait_kind CPTK_IS_CONVERTIBLE_TO, CPTK_IS_EMPTY, CPTK_IS_ENUM, + CPTK_IS_FINAL, CPTK_IS_LITERAL_TYPE, CPTK_IS_POD, CPTK_IS_POLYMORPHIC, Index: cp/parser.c =================================================================== --- cp/parser.c (revision 181967) +++ cp/parser.c (working copy) @@ -3854,6 +3854,7 @@ cp_parser_translation_unit (cp_parser* p __is_convertible_to ( type-id , type-id ) __is_empty ( type-id ) __is_enum ( type-id ) + __is_final ( type-id ) __is_literal_type ( type-id ) __is_pod ( type-id ) __is_polymorphic ( type-id ) @@ -4199,6 +4200,7 @@ cp_parser_primary_expression (cp_parser case RID_IS_CONVERTIBLE_TO: case RID_IS_EMPTY: case RID_IS_ENUM: + case RID_IS_FINAL: case RID_IS_LITERAL_TYPE: case RID_IS_POD: case RID_IS_POLYMORPHIC: @@ -7868,6 +7870,9 @@ cp_parser_trait_expr (cp_parser* parser, case RID_IS_ENUM: kind = CPTK_IS_ENUM; break; + case RID_IS_FINAL: + kind = CPTK_IS_FINAL; + break; case RID_IS_LITERAL_TYPE: kind = CPTK_IS_LITERAL_TYPE; break; Index: cp/semantics.c =================================================================== --- cp/semantics.c (revision 181967) +++ cp/semantics.c (working copy) @@ -5412,6 +5412,9 @@ trait_expr_value (cp_trait_kind kind, tr case CPTK_IS_ENUM: return (type_code1 == ENUMERAL_TYPE); + case CPTK_IS_FINAL: + return (CLASS_TYPE_P (type1) && CLASSTYPE_FINAL (type1)); + case CPTK_IS_LITERAL_TYPE: return (literal_type_p (type1)); @@ -5471,6 +5474,7 @@ finish_trait_expr (cp_trait_kind kind, t || kind == CPTK_IS_CONVERTIBLE_TO || kind == CPTK_IS_EMPTY || kind == CPTK_IS_ENUM + || kind == CPTK_IS_FINAL || kind == CPTK_IS_LITERAL_TYPE || kind == CPTK_IS_POD || kind == CPTK_IS_POLYMORPHIC @@ -5511,6 +5515,7 @@ finish_trait_expr (cp_trait_kind kind, t case CPTK_HAS_VIRTUAL_DESTRUCTOR: case CPTK_IS_ABSTRACT: case CPTK_IS_EMPTY: + case CPTK_IS_FINAL: case CPTK_IS_LITERAL_TYPE: case CPTK_IS_POD: case CPTK_IS_POLYMORPHIC: Index: cp/cxx-pretty-print.c =================================================================== --- cp/cxx-pretty-print.c (revision 181967) +++ cp/cxx-pretty-print.c (working copy) @@ -2380,6 +2380,9 @@ pp_cxx_trait_expression (cxx_pretty_prin case CPTK_IS_ENUM: pp_cxx_ws_string (pp, "__is_enum"); break; + case CPTK_IS_FINAL: + pp_cxx_ws_string (pp, "__is_final"); + break; case CPTK_IS_POD: pp_cxx_ws_string (pp, "__is_pod"); break; Index: doc/extend.texi =================================================================== --- doc/extend.texi (revision 181967) +++ doc/extend.texi (working copy) @@ -15164,6 +15164,10 @@ of unknown bound. If @code{type} is a cv enumeration type ([basic.compound]) the trait is true, else it is false. +@item __is_final (type) +If @code{type} is a cv class type marked @code{final} ([class]) the trait +is true, else it is false. + @item __is_literal_type (type) If @code{type} is a literal type ([basic.types]) the trait is true, else it is false. Requires: @code{type} shall be a complete type, Index: testsuite/g++.dg/ext/is_final.C =================================================================== --- testsuite/g++.dg/ext/is_final.C (revision 0) +++ testsuite/g++.dg/ext/is_final.C (revision 0) @@ -0,0 +1,46 @@ +// PR c++/51365 +// { dg-do compile } +// { dg-options "-std=c++0x" } + +struct A { }; +static_assert( ! __is_final (A), "A not final" ); + +struct Af final { }; +static_assert( __is_final (Af), "Af is final" ); + +class B { }; +static_assert( ! __is_final (B), "B not final" ); + +class Bf final { }; +static_assert( __is_final (Bf), "Bf is final" ); + +struct C : private A, private B { }; +static_assert( ! __is_final (C), "C not final" ); + +struct Cf final : private A, private B { }; +static_assert( __is_final (Cf), "Cf is final" ); + +struct D { virtual ~D() final { } }; +static_assert( ! __is_final (D), "D not final" ); + +struct Df final { virtual ~Df() final { } }; +static_assert( __is_final (Df), "Df is final" ); + +template<typename> struct E { }; +static_assert( ! __is_final (E<int>), "E<int> not final" ); +static_assert( ! __is_final (E<Af>), "E<Af> not final" ); + +template<typename> struct Ef final { }; +static_assert( __is_final (Ef<int>), "Ef<int> is final" ); +static_assert( __is_final (Ef<A>), "Ef<A> is final" ); +static_assert( __is_final (Ef<Af>), "Ef<Af> is final" ); + +template<typename> struct F { virtual ~F() final { }; }; +static_assert( ! __is_final (F<int>), "F<int> not final" ); +static_assert( ! __is_final (F<Af>), "F<Af> not final" ); + +template<typename> struct Ff final { virtual ~Ff() final { }; }; +static_assert( __is_final (Ff<int>), "Ff<int> is final" ); +static_assert( __is_final (Ff<A>), "Ff<A> is final" ); +static_assert( __is_final (Ff<Af>), "Ff<Af> is final" ); +