c++: cleanup ctor_omit_inherited_parms [PR97268]

ctor_omit_inherited_parms was being somewhat abused.  What I'd missed
is that it checks for a base-dtor name, before proceeding with the
check.  But we ended up passing it that during cloning before we'd
completed the cloning.  It was also using DECL_ORIGIN to get to the
in-charge ctor, but we sometimes zap DECL_ABSTRACT_ORIGIN, and it ends
up processing the incoming function -- which happens to work.  so,
this breaks out a predicate that expects to get the incharge ctor, and
will tell you whether its base ctor will need to omit the parms.  We
call that directly during cloning.

Then the original fn is essentially just a wrapper, but uses
DECL_CLONED_FUNCTION to get to the in-charge ctor.  That uncovered
abuse in add_method, which was happily passing TEMPLATE_DECLs to it.
Let's not do that.  add_method itself contained a loop mostly
containing an 'if (nomatch) continue' idiom, except for a final 'if
(match) {...}'  check, which itself contained instances of the former
idiom.  I refactored that to use the former idiom throughout.  In
doing that I found a place where we'd issue an error, but then not
actually reject the new member.

	gcc/cp/
	* cp-tree.h (base_ctor_omit_inherited_parms): Declare.
	* class.c (add_method): Refactor main loop, only pass fns to
	ctor_omit_inherited_parms.
	(build_cdtor_clones): Rename bool parms.
	(clone_cdtor): Call base_ctor_omit_inherited_parms.
	* method.c (base_ctor_omit_inherited_parms): New, broken out of
	...
	(ctor_omit_inherited_parms): ... here, call it with
	DECL_CLONED_FUNCTION.
	gcc/testsuite/
	* g++.dg/inherit/pr97268.C: New.

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index c9a1f753d565419cd9bc6d4b2b28c7870de11fc7..01780fe82917c287b227333863eb1e29e268582f 100644 (file)
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -1006,10 +1006,6 @@ add_method (tree type, tree method, bool via_using)
   for (ovl_iterator iter (current_fns); iter; ++iter)
     {
       tree fn = *iter;
-      tree fn_type;
-      tree method_type;
-      tree parms1;
-      tree parms2;
 
       if (TREE_CODE (fn) != TREE_CODE (method))
        continue;
@@ -1037,10 +1033,8 @@ add_method (tree type, tree method, bool via_using)
         functions in the derived class override and/or hide member
         functions with the same name and parameter types in a base
         class (rather than conflicting).  */
-      fn_type = TREE_TYPE (fn);
-      method_type = TREE_TYPE (method);
-      parms1 = TYPE_ARG_TYPES (fn_type);
-      parms2 = TYPE_ARG_TYPES (method_type);
+      tree fn_type = TREE_TYPE (fn);
+      tree method_type = TREE_TYPE (method);
 
       /* Compare the quals on the 'this' parm.  Don't compare
         the whole types, as used functions are treated as
@@ -1055,137 +1049,149 @@ add_method (tree type, tree method, bool via_using)
              || type_memfn_rqual (fn_type) != type_memfn_rqual (method_type)))
          continue;
 
-      /* For templates, the return type and template parameters
-        must be identical.  */
-      if (TREE_CODE (fn) == TEMPLATE_DECL
-         && (!same_type_p (TREE_TYPE (fn_type),
-                           TREE_TYPE (method_type))
-             || !comp_template_parms (DECL_TEMPLATE_PARMS (fn),
-                                      DECL_TEMPLATE_PARMS (method))))
+      tree real_fn = fn;
+      tree real_method = method;
+
+      /* Templates and conversion ops must match return types.  */
+      if ((DECL_CONV_FN_P (fn) || TREE_CODE (fn) == TEMPLATE_DECL)
+         && !same_type_p (TREE_TYPE (fn_type), TREE_TYPE (method_type)))
        continue;
+      
+      /* For templates, the template parameters must be identical.  */
+      if (TREE_CODE (fn) == TEMPLATE_DECL)
+       {
+         if (!comp_template_parms (DECL_TEMPLATE_PARMS (fn),
+                                   DECL_TEMPLATE_PARMS (method)))
+           continue;
 
-      if (! DECL_STATIC_FUNCTION_P (fn))
+         real_fn = DECL_TEMPLATE_RESULT (fn);
+         real_method = DECL_TEMPLATE_RESULT (method);
+       }
+
+      tree parms1 = TYPE_ARG_TYPES (fn_type);
+      tree parms2 = TYPE_ARG_TYPES (method_type);
+      if (! DECL_STATIC_FUNCTION_P (real_fn))
        parms1 = TREE_CHAIN (parms1);
-      if (! DECL_STATIC_FUNCTION_P (method))
+      if (! DECL_STATIC_FUNCTION_P (real_method))
        parms2 = TREE_CHAIN (parms2);
 
-      /* Bring back parameters omitted from an inherited ctor.  */
-      if (ctor_omit_inherited_parms (fn))
-       parms1 = FUNCTION_FIRST_USER_PARMTYPE (DECL_ORIGIN (fn));
-      if (ctor_omit_inherited_parms (method))
-       parms2 = FUNCTION_FIRST_USER_PARMTYPE (DECL_ORIGIN (method));
+      /* Bring back parameters omitted from an inherited ctor.  The
+        method and the function can have different omittedness.  */
+      if (ctor_omit_inherited_parms (real_fn))
+       parms1 = FUNCTION_FIRST_USER_PARMTYPE (DECL_CLONED_FUNCTION (real_fn));
+      if (ctor_omit_inherited_parms (real_method))
+       parms2 = (FUNCTION_FIRST_USER_PARMTYPE
+                 (DECL_CLONED_FUNCTION (real_method)));
 
-      if (compparms (parms1, parms2)
-         && (!DECL_CONV_FN_P (fn)
-             || same_type_p (TREE_TYPE (fn_type),
-                             TREE_TYPE (method_type))))
-       {
-          if (!equivalently_constrained (fn, method))
-           {
-             if (processing_template_decl)
-               /* We can't check satisfaction in dependent context, wait until
-                  the class is instantiated.  */
-               continue;
-
-             special_function_kind sfk = special_memfn_p (method);
+      if (!compparms (parms1, parms2))
+       continue;
 
-             if (sfk == sfk_none
-                 || DECL_INHERITED_CTOR (fn)
-                 || TREE_CODE (fn) == TEMPLATE_DECL)
-               /* Member function templates and non-special member functions
-                  coexist if they are not equivalently constrained.  A member
-                  function is not hidden by an inherited constructor.  */
-               continue;
+      if (!equivalently_constrained (fn, method))
+       {
+         if (processing_template_decl)
+           /* We can't check satisfaction in dependent context, wait until
+              the class is instantiated.  */
+           continue;
 
-             /* P0848: For special member functions, deleted, unsatisfied, or
-                less constrained overloads are ineligible.  We implement this
-                by removing them from CLASSTYPE_MEMBER_VEC.  Destructors don't
-                use the notion of eligibility, and the selected destructor can
-                be deleted, but removing unsatisfied or less constrained
-                overloads has the same effect as overload resolution.  */
-             bool dtor = (sfk == sfk_destructor);
-             if (losem == -1)
-               losem = ((!dtor && DECL_DELETED_FN (method))
-                        || !constraints_satisfied_p (method));
-             bool losef = ((!dtor && DECL_DELETED_FN (fn))
-                           || !constraints_satisfied_p (fn));
-             int win;
-             if (losem || losef)
-               win = losem - losef;
-             else
-               win = more_constrained (fn, method);
-             if (win > 0)
-               /* Leave FN in the method vec, discard METHOD.  */
-               return false;
-             else if (win < 0)
-               {
-                 /* Remove FN, add METHOD.  */
-                 current_fns = iter.remove_node (current_fns);
-                 continue;
-               }
-             else
-               /* Let them coexist for now.  */
-               continue;
-           }
+         special_function_kind sfk = special_memfn_p (method);
 
-         /* If these are versions of the same function, process and
-            move on.  */
-         if (TREE_CODE (fn) == FUNCTION_DECL
-             && maybe_version_functions (method, fn, true))
+         if (sfk == sfk_none
+             || DECL_INHERITED_CTOR (fn)
+             || TREE_CODE (fn) == TEMPLATE_DECL)
+           /* Member function templates and non-special member functions
+              coexist if they are not equivalently constrained.  A member
+              function is not hidden by an inherited constructor.  */
            continue;
 
-         if (DECL_INHERITED_CTOR (method))
-           {
-             if (DECL_INHERITED_CTOR (fn))
-               {
-                 tree basem = DECL_INHERITED_CTOR_BASE (method);
-                 tree basef = DECL_INHERITED_CTOR_BASE (fn);
-                 if (flag_new_inheriting_ctors)
-                   {
-                     if (basem == basef)
-                       {
-                         /* Inheriting the same constructor along different
-                            paths, combine them.  */
-                         SET_DECL_INHERITED_CTOR
-                           (fn, ovl_make (DECL_INHERITED_CTOR (method),
-                                          DECL_INHERITED_CTOR (fn)));
-                         /* And discard the new one.  */
-                         return false;
-                       }
-                     else
-                       /* Inherited ctors can coexist until overload
-                          resolution.  */
-                       continue;
-                   }
-                 error_at (DECL_SOURCE_LOCATION (method),
-                           "%q#D conflicts with version inherited from %qT",
-                           method, basef);
-                 inform (DECL_SOURCE_LOCATION (fn),
-                         "version inherited from %qT declared here",
-                         basef);
-               }
-             /* Otherwise defer to the other function.  */
-             return false;
-           }
-
-         if (via_using)
-           /* Defer to the local function.  */
+         /* P0848: For special member functions, deleted, unsatisfied, or
+            less constrained overloads are ineligible.  We implement this
+            by removing them from CLASSTYPE_MEMBER_VEC.  Destructors don't
+            use the notion of eligibility, and the selected destructor can
+            be deleted, but removing unsatisfied or less constrained
+            overloads has the same effect as overload resolution.  */
+         bool dtor = (sfk == sfk_destructor);
+         if (losem == -1)
+           losem = ((!dtor && DECL_DELETED_FN (method))
+                    || !constraints_satisfied_p (method));
+         bool losef = ((!dtor && DECL_DELETED_FN (fn))
+                       || !constraints_satisfied_p (fn));
+         int win;
+         if (losem || losef)
+           win = losem - losef;
+         else
+           win = more_constrained (fn, method);
+         if (win > 0)
+           /* Leave FN in the method vec, discard METHOD.  */
            return false;
-         else if (flag_new_inheriting_ctors
-                  && DECL_INHERITED_CTOR (fn))
+         else if (win < 0)
            {
-             /* Remove the inherited constructor.  */
+             /* Remove FN, add METHOD.  */
              current_fns = iter.remove_node (current_fns);
              continue;
            }
          else
+           /* Let them coexist for now.  */
+           continue;
+       }
+
+      /* If these are versions of the same function, process and
+        move on.  */
+      if (TREE_CODE (fn) == FUNCTION_DECL
+         && maybe_version_functions (method, fn, true))
+       continue;
+
+      if (DECL_INHERITED_CTOR (method))
+       {
+         if (!DECL_INHERITED_CTOR (fn))
+           /* Defer to the other function.  */
+           return false;
+           
+         tree basem = DECL_INHERITED_CTOR_BASE (method);
+         tree basef = DECL_INHERITED_CTOR_BASE (fn);
+         if (flag_new_inheriting_ctors)
            {
-             error_at (DECL_SOURCE_LOCATION (method),
-                       "%q#D cannot be overloaded with %q#D", method, fn);
-             inform (DECL_SOURCE_LOCATION (fn),
-                     "previous declaration %q#D", fn);
-             return false;
+             if (basem == basef)
+               {
+                 /* Inheriting the same constructor along different
+                    paths, combine them.  */
+                 SET_DECL_INHERITED_CTOR
+                   (fn, ovl_make (DECL_INHERITED_CTOR (method),
+                                  DECL_INHERITED_CTOR (fn)));
+                 /* And discard the new one.  */
+                 return false;
+               }
+             else
+               /* Inherited ctors can coexist until overload
+                  resolution.  */
+               continue;
            }
+
+         error_at (DECL_SOURCE_LOCATION (method),
+                   "%q#D conflicts with version inherited from %qT",
+                   method, basef);
+         inform (DECL_SOURCE_LOCATION (fn),
+                 "version inherited from %qT declared here",
+                 basef);
+         return false;
+       }
+
+      if (via_using)
+       /* Defer to the local function.  */
+       return false;
+      else if (flag_new_inheriting_ctors
+              && DECL_INHERITED_CTOR (fn))
+       {
+         /* Remove the inherited constructor.  */
+         current_fns = iter.remove_node (current_fns);
+         continue;
+       }
+      else
+       {
+         error_at (DECL_SOURCE_LOCATION (method),
+                   "%q#D cannot be overloaded with %q#D", method, fn);
+         inform (DECL_SOURCE_LOCATION (fn),
+                 "previous declaration %q#D", fn);
+         return false;
        }
     }
 
@@ -4892,7 +4898,7 @@ build_clone (tree fn, tree name, bool need_vtt_parm_p,
    will be inserted onto DECL_CHAIN of FN.  */
 
 static unsigned
-build_cdtor_clones (tree fn, bool needs_vtt_parm_p, bool omit_inherited_parms_p)
+build_cdtor_clones (tree fn, bool needs_vtt_p, bool base_omits_inherited_p)
 {
   unsigned count = 0;
 
@@ -4901,8 +4907,8 @@ build_cdtor_clones (tree fn, bool needs_vtt_parm_p, bool omit_inherited_parms_p)
       /* For each constructor, we need two variants: an in-charge version
         and a not-in-charge version.  */
       build_clone (fn, complete_ctor_identifier, false, false);
-      build_clone (fn, base_ctor_identifier, needs_vtt_parm_p,
-                  omit_inherited_parms_p);
+      build_clone (fn, base_ctor_identifier, needs_vtt_p,
+                  base_omits_inherited_p);
       count += 2;
     }
   else
@@ -4924,7 +4930,7 @@ build_cdtor_clones (tree fn, bool needs_vtt_parm_p, bool omit_inherited_parms_p)
          count++;
        }
       build_clone (fn, complete_dtor_identifier, false, false);
-      build_clone (fn, base_dtor_identifier, needs_vtt_parm_p, false);
+      build_clone (fn, base_dtor_identifier, needs_vtt_p, false);
       count += 2;
     }
 
@@ -4948,9 +4954,10 @@ clone_cdtor (tree fn, bool update_methods)
 
   /* Base ctor omits inherited parms it needs a vttparm and inherited
      from a virtual nase ctor.  */
-  bool omit_inherited = ctor_omit_inherited_parms (fn);
+  bool base_omits_inherited = (DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (fn)
+                              && base_ctor_omit_inherited_parms (fn));
 
-  unsigned count = build_cdtor_clones (fn, vtt, omit_inherited);
+  unsigned count = build_cdtor_clones (fn, vtt, base_omits_inherited);
 
   /* Note that this is an abstract function that is never emitted.  */
   DECL_ABSTRACT_P (fn) = true;

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 9f948aee2d82b83a9fef0b227d034755f3371e15..43e0c18ec03cfcf5d30b4792197d8b7c86b915b3 100644 (file)
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6764,6 +6764,7 @@ extern tree get_default_ctor                      (tree);
 extern tree get_dtor                           (tree, tsubst_flags_t);
 extern tree strip_inheriting_ctors             (tree);
 extern tree inherited_ctor_binfo               (tree);
+extern bool base_ctor_omit_inherited_parms     (tree);
 extern bool ctor_omit_inherited_parms          (tree);
 extern tree locate_ctor                                (tree);
 extern tree implicitly_declare_fn               (special_function_kind, tree,

diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 1058fd05a7d20481d977309a28f366c70e30d529..6e4c5f7e83b8fb6ff4ae691bb1b53d3b59fb7879 100644 (file)
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -551,31 +551,51 @@ inherited_ctor_binfo (tree fndecl)
   return inherited_ctor_binfo (binfo, fndecl);
 }
 
-/* True if we should omit all user-declared parameters from constructor FN,
-   because it is a base clone of a ctor inherited from a virtual base.  */
+
+/* True if we should omit all user-declared parameters from a base
+   construtor built from complete constructor FN.
+   That's when the ctor is inherited from a virtual base.  */
 
 bool
-ctor_omit_inherited_parms (tree fn)
+base_ctor_omit_inherited_parms (tree comp_ctor)
 {
+  gcc_checking_assert (DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (comp_ctor));
+
   if (!flag_new_inheriting_ctors)
     /* We only optimize away the parameters in the new model.  */
     return false;
-  if (!DECL_BASE_CONSTRUCTOR_P (fn)
-      || !CLASSTYPE_VBASECLASSES (DECL_CONTEXT (fn)))
+
+  if (!CLASSTYPE_VBASECLASSES (DECL_CONTEXT (comp_ctor)))
     return false;
 
-  if (FUNCTION_FIRST_USER_PARMTYPE (DECL_ORIGIN (fn)) == void_list_node)
+  if (FUNCTION_FIRST_USER_PARMTYPE (comp_ctor) == void_list_node)
     /* No user-declared parameters to omit.  */
     return false;
 
-  tree binfo = inherited_ctor_binfo (fn);
-  for (; binfo; binfo = BINFO_INHERITANCE_CHAIN (binfo))
+  for (tree binfo = inherited_ctor_binfo (comp_ctor);
+       binfo;
+       binfo = BINFO_INHERITANCE_CHAIN (binfo))
     if (BINFO_VIRTUAL_P (binfo))
       return true;
 
   return false;
 }
 
+
+/* True if we should omit all user-declared parameters from constructor FN,
+   because it is a base clone of a ctor inherited from a virtual base.  */
+
+bool
+ctor_omit_inherited_parms (tree fn)
+{
+  gcc_checking_assert (TREE_CODE (fn) == FUNCTION_DECL);
+
+  if (!DECL_BASE_CONSTRUCTOR_P (fn))
+    return false;
+
+  return base_ctor_omit_inherited_parms (DECL_CLONED_FUNCTION (fn));
+}
+
 /* True iff constructor(s) INH inherited into BINFO initializes INIT_BINFO.
    This can be true for multiple virtual bases as well as one direct
    non-virtual base.  */

diff --git a/gcc/testsuite/g++.dg/inherit/pr97268.C b/gcc/testsuite/g++.dg/inherit/pr97268.C
new file mode 100644 (file)
index 0000000..79a809c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/inherit/pr97268.C
@@ -0,0 +1,60 @@
+// { dg-do compile { target c++11 } }
+// { dg-additional-options -Wall }
+// PR 97268, ICE due to broken inherited-from-virtual base-ctor
+class Handle {
+public:
+    explicit Handle(char const *const &) { }
+    ~Handle() {}
+    Handle(const Handle &) = delete;
+    Handle &operator=(const Handle &) = delete;
+
+protected:
+    int lasterr = 0;
+
+};
+
+struct ObjectBase {
+    ~ObjectBase() {}
+
+protected:
+    explicit ObjectBase(const char *lc_, int ln_, Handle &h, unsigned) 
+            :  handle(h) { }
+
+protected:
+
+    Handle &handle;
+};
+
+template <bool CACHED>
+struct Object : virtual public ObjectBase {
+    explicit Object(const char *lc_, int ln_, Handle &env);
+
+protected:
+    using ObjectBase::ObjectBase;
+
+};
+
+class BetterObjectBase : virtual public ObjectBase {
+protected:
+    BetterObjectBase(const char *lc_, int ln_, Handle &env)
+            : ObjectBase("", 0, env, 0) {}
+
+};
+
+template <bool CACHED>
+class BetterObject : public Object<CACHED>, public BetterObjectBase {
+public:
+    BetterObject(Handle &env)
+      : ObjectBase("", 0, env, 0)
+      , Object<CACHED>("", 0, env, 0)
+      , BetterObjectBase("", 0, env) {} // { dg-error "use of deleted function" }
+
+};
+
+int main() {
+    Handle h("handle");
+
+    BetterObject<true> B(h);
+
+    return 0;
+}