| Message ID | 0adea123-1e7d-293c-25a2-ee290fddf678@acm.org |
|---|---|
| State | New |
| Headers | show |
ping? On 01/11/2017 10:53 AM, Nathan Sidwell wrote: > On 01/04/2017 12:53 AM, Jason Merrill wrote: > >> Hmm, that seems like where the problem is. We shouldn't try to >> instantiate the inheriting constructor until we've already chosen the >> base constructor; in the new model the inheriting constructor is just an >> implementation detail. > > Oh what fun. This testcase behaves differently for C++17, C++11 > -fnew-inheriting-ctors and C++11 -fno-new-inheriting-ctors compilation > modes. > > Firstly, unpatched G++ is fine in C++17 mode, because: > /* In C++17, "If the initializer expression is a prvalue and the > cv-unqualified version of the source type is the same class as the > class > of the destination, the initializer expression is used to > initialize the > destination object." Handle that here to avoid doing overload > resolution. */ > and inside that we have: > > /* FIXME P0135 doesn't say how to handle direct initialization from a > type with a suitable conversion operator. Let's handle it like > copy-initialization, but allowing explict conversions. */ > > That conversion lookup short-circuits the subsequent overload resolution > that would otherwise explode. > > Otherwise, with -fnew-inheriting-ctors, you are indeed correct. There > needs to be a call to strip_inheriting_ctors in deduce_inheriting_ctor. > > With -fno-new-inheriting-ctors we need the original patch I posted > (included herein). I suppose we might be able to remove the assert from > strip_inheriting_ctors and always call that from deduce_inheriting_ctor, > but that seems a bad idea to me. > > I was unable to produce a c++17 testcase that triggered this problem by > avoiding the above-mentioned overload resolution short-circuiting. > > As -fnew-inheriting-ctors is a mangling-affecting flag, I guess we're > stuck with it for the foreseable future. > > ok? > > nathan > -- Nathan Sidwell
On Wed, Jan 11, 2017 at 10:53 AM, Nathan Sidwell <nathan@acm.org> wrote: > On 01/04/2017 12:53 AM, Jason Merrill wrote: > >> Hmm, that seems like where the problem is. We shouldn't try to >> instantiate the inheriting constructor until we've already chosen the >> base constructor; in the new model the inheriting constructor is just an >> implementation detail. > > Oh what fun. This testcase behaves differently for C++17, C++11 > -fnew-inheriting-ctors and C++11 -fno-new-inheriting-ctors compilation > modes. > > Firstly, unpatched G++ is fine in C++17 mode, because: > /* In C++17, "If the initializer expression is a prvalue and the > cv-unqualified version of the source type is the same class as the > class > of the destination, the initializer expression is used to initialize > the > destination object." Handle that here to avoid doing overload > resolution. */ > and inside that we have: > > /* FIXME P0135 doesn't say how to handle direct initialization from a > type with a suitable conversion operator. Let's handle it like > copy-initialization, but allowing explict conversions. */ > > That conversion lookup short-circuits the subsequent overload resolution > that would otherwise explode. > > Otherwise, with -fnew-inheriting-ctors, you are indeed correct. There needs > to be a call to strip_inheriting_ctors in deduce_inheriting_ctor. That doesn't seem quite right; in deducing the inheriting ctor we are interested in what it actually calls, so we don't want to strip. I was thinking about changing when we do that deduction: we shouldn't be calling deduce_inheriting_ctor until we actually know we're calling this inheriting ctor. I was thinking that would mean removing the code in fn_type_unification with the comment /* After doing deduction with the inherited constructor, actually return an instantiation of the inheriting constructor. */ and then looking up the inheriting constructor somehow in build_over_call. But that gets to be a big change. Something smaller would be moving the call to deduce_inheriting_ctor to build_over_call; we can get away with that because calling is the only way to refer to a constructor. What do you think of this approach? commit 56586a488a27f2d5b502bd35aaec7225d0fb1d31 Author: Jason Merrill <jason@redhat.com> Date: Wed Jan 25 16:52:28 2017 -0500 deduce-latediff --git a/gcc/cp/call.c b/gcc/cp/call.c index a78e1a9..99c51f3 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -7581,6 +7581,11 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) joust (cand, w->loser, 1, complain); } + /* OK, we're actually calling this inherited constructor; set its deletedness + appropriately. */ + if (DECL_INHERITED_CTOR (fn)) + deduce_inheriting_ctor (fn); + /* Make =delete work with SFINAE. */ if (DECL_DELETED_FN (fn) && !(complain & tf_error)) return error_mark_node; diff --git a/gcc/cp/class.c b/gcc/cp/class.c index b7c26a1..03a9730 100644 --- a/gcc/cp/class.c +++ b/gcc/cp/class.c @@ -1197,8 +1197,6 @@ add_method (tree type, tree method, tree using_decl) SET_DECL_INHERITED_CTOR (fn, ovl_cons (DECL_INHERITED_CTOR (method), DECL_INHERITED_CTOR (fn))); - /* Adjust deletedness and such. */ - deduce_inheriting_ctor (fn); /* And discard the new one. */ return false; } diff --git a/gcc/cp/method.c b/gcc/cp/method.c index 5b366f0..e80b806 100644 --- a/gcc/cp/method.c +++ b/gcc/cp/method.c @@ -1855,6 +1855,7 @@ explain_implicit_non_constexpr (tree decl) void deduce_inheriting_ctor (tree decl) { + decl = DECL_ORIGIN (decl); gcc_assert (DECL_INHERITED_CTOR (decl)); tree spec; bool trivial, constexpr_, deleted; @@ -1868,6 +1869,13 @@ deduce_inheriting_ctor (tree decl) deleted = true; DECL_DELETED_FN (decl) = deleted; TREE_TYPE (decl) = build_exception_variant (TREE_TYPE (decl), spec); + + tree clone; + FOR_EACH_CLONE (clone, decl) + { + DECL_DELETED_FN (clone) = deleted; + TREE_TYPE (clone) = build_exception_variant (TREE_TYPE (clone), spec); + } } /* Implicitly declare the special function indicated by KIND, as a @@ -1968,10 +1976,10 @@ implicitly_declare_fn (special_function_kind kind, tree type, bool trivial_p = false; - if (inherited_ctor && TREE_CODE (inherited_ctor) == TEMPLATE_DECL) + if (inherited_ctor) { - /* For an inheriting constructor template, just copy these flags from - the inherited constructor template for now. */ + /* For an inheriting constructor, just copy these flags from the + inherited constructor until deduce_inheriting_ctor. */ raises = TYPE_RAISES_EXCEPTIONS (TREE_TYPE (inherited_ctor)); deleted_p = DECL_DELETED_FN (inherited_ctor); constexpr_p = DECL_DECLARED_CONSTEXPR_P (inherited_ctor); diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 537d107..c96ceed 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -12358,8 +12358,6 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) maybe_retrofit_in_chrg (r); if (DECL_CONSTRUCTOR_P (r)) grok_ctor_properties (ctx, r); - if (DECL_INHERITED_CTOR (r)) - deduce_inheriting_ctor (r); /* If this is an instantiation of a member template, clone it. If it isn't, that'll be handled by clone_constructors_and_destructors. */ diff --git a/gcc/testsuite/g++.dg/cpp0x/pr78771-new.C b/gcc/testsuite/g++.dg/cpp0x/pr78771-new.C new file mode 100644 index 0000000..f489f86 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/pr78771-new.C @@ -0,0 +1,28 @@ +// PR c++/78771 +// { dg-do compile { target c++11 } } +// { dg-additional-options "-fnew-inheriting-ctors" } + +// ICE instantiating a deleted inherited ctor + +struct Base +{ + template <typename U> Base (U); + + Base (int); +}; + +struct Derived; + +struct Middle : Base +{ + using Base::Base; + + Middle (Derived); +}; + +struct Derived : Middle +{ + using Middle::Middle; +}; + +Middle::Middle (Derived) : Middle (0) {} diff --git a/gcc/testsuite/g++.dg/cpp0x/pr78771-old.C b/gcc/testsuite/g++.dg/cpp0x/pr78771-old.C new file mode 100644 index 0000000..b723b11 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/pr78771-old.C @@ -0,0 +1,28 @@ +// PR c++/78771 +// { dg-do compile { target c++11 } } +// { dg-additional-options "-fno-new-inheriting-ctors" } + +// ICE instantiating a deleted inherited ctor + +struct Base +{ + template <typename U> Base (U); + + Base (int); +}; + +struct Derived; + +struct Middle : Base +{ + using Base::Base; + + Middle (Derived); +}; + +struct Derived : Middle +{ + using Middle::Middle; +}; + +Middle::Middle (Derived) : Middle (0) {} diff --git a/gcc/testsuite/g++.dg/cpp1z/pr78771.C b/gcc/testsuite/g++.dg/cpp1z/pr78771.C new file mode 100644 index 0000000..9178494 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/pr78771.C @@ -0,0 +1,27 @@ +// PR c++/78771 +// { dg-options -std=c++1z } + +// ICE instantiating a deleted inherited ctor + +struct Base +{ + template <typename U> Base (U); + + Base (int); +}; + +struct Derived; + +struct Middle : Base +{ + using Base::Base; + + Middle (Derived); +}; + +struct Derived : Middle +{ + using Middle::Middle; +}; + +Middle::Middle (Derived) : Middle (0) {}
On 01/25/2017 05:09 PM, Jason Merrill wrote: > Something smaller would be moving the call to deduce_inheriting_ctor > to build_over_call; we can get away with that because calling is the > only way to refer to a constructor. What do you think of this > approach? LGTM, thanks! nathan -- Nathan Sidwell
2017-01-11 Nathan Sidwell <nathan@acm.org> PR c++/78771 * pt.c (instantiate_template_1): Check for recursive instantiation of inheriting constructor when not new-inheriting-ctor. * method.c (deduce_inheriting_ctor): Use originating ctor when new-inheriting-ctor. PR c++/78771 * g++.dg/cpp0x/pr78771-old.C: New. * g++.dg/cpp0x/pr78771-new.C: New. * g++.dg/cpp1z/pr78771.C: New. Index: cp/method.c =================================================================== --- cp/method.c (revision 244314) +++ cp/method.c (working copy) @@ -1858,11 +1858,15 @@ deduce_inheriting_ctor (tree decl) gcc_assert (DECL_INHERITED_CTOR (decl)); tree spec; bool trivial, constexpr_, deleted; + + tree inherited = DECL_INHERITED_CTOR (decl); + if (flag_new_inheriting_ctors) + inherited = strip_inheriting_ctors (inherited); + synthesized_method_walk (DECL_CONTEXT (decl), sfk_inheriting_constructor, false, &spec, &trivial, &deleted, &constexpr_, - /*diag*/false, - DECL_INHERITED_CTOR (decl), - FUNCTION_FIRST_USER_PARMTYPE (decl)); + /*diag=*/false, + inherited, FUNCTION_FIRST_USER_PARMTYPE (decl)); if (TREE_CODE (inherited_ctor_binfo (decl)) != TREE_BINFO) /* Inherited the same constructor from different base subobjects. */ deleted = true; Index: cp/pt.c =================================================================== --- cp/pt.c (revision 244314) +++ cp/pt.c (working copy) @@ -17963,10 +17963,22 @@ instantiate_template_1 (tree tmpl, tree if (spec == error_mark_node) return error_mark_node; + /* If this is an inherited ctor, we can recursively clone it + when deducing the validity of the ctor. But we won't have + cloned the function yet, so do it now. We'll redo this + later, but any recursive information learnt here can't + change the validity. */ + if (!TREE_CHAIN (spec)) + { + gcc_assert (!flag_new_inheriting_ctors + && DECL_INHERITED_CTOR (spec)); + clone_function_decl (spec, /*update_method_vec_p=*/0); + } /* Look for the clone. */ FOR_EACH_CLONE (clone, spec) if (DECL_NAME (clone) == DECL_NAME (tmpl)) return clone; + /* We should always have found the clone by now. */ gcc_unreachable (); return NULL_TREE; Index: testsuite/g++.dg/cpp0x/pr78771-new.C =================================================================== --- testsuite/g++.dg/cpp0x/pr78771-new.C (revision 0) +++ testsuite/g++.dg/cpp0x/pr78771-new.C (working copy) @@ -0,0 +1,28 @@ +// PR c++/78771 +// { dg-do compile { target c++11 } } +// { dg-additional-options "-fnew-inheriting-ctors" } + +// ICE instantiating a deleted inherited ctor + +struct Base +{ + template <typename U> Base (U); + + Base (int); +}; + +struct Derived; + +struct Middle : Base +{ + using Base::Base; + + Middle (Derived); +}; + +struct Derived : Middle +{ + using Middle::Middle; +}; + +Middle::Middle (Derived) : Middle (0) {} Index: testsuite/g++.dg/cpp0x/pr78771-old.C =================================================================== --- testsuite/g++.dg/cpp0x/pr78771-old.C (revision 0) +++ testsuite/g++.dg/cpp0x/pr78771-old.C (working copy) @@ -0,0 +1,28 @@ +// PR c++/78771 +// { dg-do compile { target c++11 } } +// { dg-additional-options "-fno-new-inheriting-ctors" } + +// ICE instantiating a deleted inherited ctor + +struct Base +{ + template <typename U> Base (U); + + Base (int); +}; + +struct Derived; + +struct Middle : Base +{ + using Base::Base; + + Middle (Derived); +}; + +struct Derived : Middle +{ + using Middle::Middle; +}; + +Middle::Middle (Derived) : Middle (0) {} Index: testsuite/g++.dg/cpp1z/pr78771.C =================================================================== --- testsuite/g++.dg/cpp1z/pr78771.C (revision 0) +++ testsuite/g++.dg/cpp1z/pr78771.C (working copy) @@ -0,0 +1,27 @@ +// PR c++/78771 +// { dg-options -std=c++1z } + +// ICE instantiating a deleted inherited ctor + +struct Base +{ + template <typename U> Base (U); + + Base (int); +}; + +struct Derived; + +struct Middle : Base +{ + using Base::Base; + + Middle (Derived); +}; + +struct Derived : Middle +{ + using Middle::Middle; +}; + +Middle::Middle (Derived) : Middle (0) {}