Hi,

this is an update on C++/54191, where in a number of cases, having to do with inaccessible bases, we don't handle correctly access control under SFINAE.

The attached draft patch p fixes a number of tests (and passes regression testing), where we are currently emitting inaccessible base diagnostics from lookup_base instead of properly handling the failure in SFINAE. As you can see in the draft, after a number of tries, I'm simply adding a tsubst_flags_t parameter to lookup_base: obviously something is redundant with base_access, but knowing that a returned error_mark_node in every case means that the base is either ambiguous or inaccessible (when we error out outside SFINAE) is *so* convenient! For comparison, the existing get_delta_difference_1 shows an alternate approach, quite contorted IMHO. But these are just implementation details...

More interesting are the cases, commented out in the attached 54191.C, which we still get wrong also with the patch applied. In those cases we do *not* currently produce any inaccessible base diagnostics, we simply mishandle the SFINAE, apparently we don't perform any form of access control. I'm really looking for help about this set of 5 tests (4 braced casts + conditional operator)

Thanks!
Paolo.

///////////////////////////

Index: typeck.c
===================================================================
--- typeck.c    (revision 190207)
+++ typeck.c    (working copy)
@@ -2246,8 +2246,8 @@ build_class_member_access_expr (tree object, tree
          tree binfo;
          base_kind kind;
 
-         binfo = lookup_base (access_path ? access_path : object_type,
-                              member_scope, ba_unique,  &kind);
+         binfo = lookup_base_sfinae (access_path ? access_path : object_type,
+                                     member_scope, ba_unique, &kind, complain);
          if (binfo == error_mark_node)
            return error_mark_node;
 
@@ -2630,7 +2630,8 @@ finish_class_member_access_expr (tree object, tree
            }
 
          /* Find the base of OBJECT_TYPE corresponding to SCOPE.  */
-         access_path = lookup_base (object_type, scope, ba_check, NULL);
+         access_path = lookup_base_sfinae (object_type, scope, ba_check,
+                                           NULL, complain);
          if (access_path == error_mark_node)
            return error_mark_node;
          if (!access_path)
@@ -3150,8 +3151,8 @@ get_member_function_from_ptrfunc (tree *instance_p
       if (!same_type_ignoring_top_level_qualifiers_p
          (basetype, TREE_TYPE (TREE_TYPE (instance_ptr))))
        {
-         basetype = lookup_base (TREE_TYPE (TREE_TYPE (instance_ptr)),
-                                 basetype, ba_check, NULL);
+         basetype = lookup_base_sfinae (TREE_TYPE (TREE_TYPE (instance_ptr)),
+                                        basetype, ba_check, NULL, complain);
          instance_ptr = build_base_path (PLUS_EXPR, instance_ptr, basetype,
                                          1, complain);
          if (instance_ptr == error_mark_node)
@@ -5995,9 +5996,9 @@ build_static_cast_1 (tree type, tree expr, bool c_
         ambiguity.  However, if this is a static_cast being performed
         because the user wrote a C-style cast, then accessibility is
         not considered.  */
-      base = lookup_base (TREE_TYPE (type), intype,
-                         c_cast_p ? ba_unique : ba_check,
-                         NULL);
+      base = lookup_base_sfinae (TREE_TYPE (type), intype,
+                                c_cast_p ? ba_unique : ba_check,
+                                NULL, complain);
 
       /* Convert from "B*" to "D*".  This function will check that "B"
         is not a virtual base of "D".  */
@@ -6119,9 +6120,9 @@ build_static_cast_1 (tree type, tree expr, bool c_
          && check_for_casting_away_constness (intype, type, STATIC_CAST_EXPR,
                                               complain))
        return error_mark_node;
-      base = lookup_base (TREE_TYPE (type), TREE_TYPE (intype),
-                         c_cast_p ? ba_unique : ba_check,
-                         NULL);
+      base = lookup_base_sfinae (TREE_TYPE (type), TREE_TYPE (intype),
+                                c_cast_p ? ba_unique : ba_check,
+                                NULL, complain);
       expr = build_base_path (MINUS_EXPR, expr, base, /*nonnull=*/false,
                              complain);
       return cp_fold_convert(type, expr);
@@ -7181,16 +7182,11 @@ get_delta_difference_1 (tree from, tree to, bool c
 {
   tree binfo;
   base_kind kind;
-  base_access access = c_cast_p ? ba_unique : ba_check;
 
-  /* Note: ba_quiet does not distinguish between access control and
-     ambiguity.  */
-  if (!(complain & tf_error))
-    access |= ba_quiet;
+  binfo = lookup_base_sfinae (to, from, c_cast_p ? ba_unique : ba_check,
+                             &kind, complain);
 
-  binfo = lookup_base (to, from, access, &kind);
-
-  if (kind == bk_inaccessible || kind == bk_ambig)
+  if (binfo == error_mark_node)
     {
       if (!(complain & tf_error))
        return error_mark_node;
Index: class.c
===================================================================
--- class.c     (revision 190207)
+++ class.c     (working copy)
@@ -274,8 +274,8 @@ build_base_path (enum tree_code code,
         a static member function, so we do it now.  */
       if (complain & tf_error)
        {
-         tree base = lookup_base (probe, BINFO_TYPE (d_binfo),
-                                  ba_unique, NULL);
+         tree base = lookup_base_sfinae (probe, BINFO_TYPE (d_binfo),
+                                         ba_unique, NULL, complain);
          gcc_assert (base == error_mark_node);
        }
       return error_mark_node;
@@ -557,9 +557,8 @@ convert_to_base (tree object, tree type, bool chec
   access = check_access ? ba_check : ba_unique;
   if (!(complain & tf_error))
     access |= ba_quiet;
-  binfo = lookup_base (object_type, type,
-                      access,
-                      NULL);
+  binfo = lookup_base_sfinae (object_type, type,
+                             access, NULL, complain);
   if (!binfo || binfo == error_mark_node)
     return error_mark_node;
 
Index: rtti.c
===================================================================
--- rtti.c      (revision 190207)
+++ rtti.c      (working copy)
@@ -614,8 +614,8 @@ build_dynamic_cast_1 (tree type, tree expr, tsubst
   {
     tree binfo;
 
-    binfo = lookup_base (TREE_TYPE (exprtype), TREE_TYPE (type),
-                        ba_check, NULL);
+    binfo = lookup_base_sfinae (TREE_TYPE (exprtype), TREE_TYPE (type),
+                               ba_check, NULL, complain);
 
     if (binfo)
       {
Index: typeck2.c
===================================================================
--- typeck2.c   (revision 190207)
+++ typeck2.c   (working copy)
@@ -1600,7 +1600,7 @@ build_m_component_ref (tree datum, tree component,
     }
   else
     {
-      binfo = lookup_base (objtype, ctype, ba_check, NULL);
+      binfo = lookup_base_sfinae (objtype, ctype, ba_check, NULL, complain);
 
       if (!binfo)
        {
Index: call.c
===================================================================
--- call.c      (revision 190207)
+++ call.c      (working copy)
@@ -6616,8 +6616,9 @@ build_over_call (struct z_candidate *cand, int fla
       /* If fn was found by a using declaration, the conversion path
         will be to the derived class, not the base declaring fn. We
         must convert from derived to base.  */
-      base_binfo = lookup_base (TREE_TYPE (TREE_TYPE (converted_arg)),
-                               TREE_TYPE (parmtype), ba_unique, NULL);
+      base_binfo = lookup_base_sfinae (TREE_TYPE (TREE_TYPE (converted_arg)),
+                                      TREE_TYPE (parmtype), ba_unique,
+                                      NULL, complain);
       converted_arg = build_base_path (PLUS_EXPR, converted_arg,
                                       base_binfo, 1, complain);
 
@@ -6859,9 +6860,9 @@ build_over_call (struct z_candidate *cand, int fla
   if (DECL_VINDEX (fn) && (flags & LOOKUP_NONVIRTUAL) == 0)
     {
       tree t;
-      tree binfo = lookup_base (TREE_TYPE (TREE_TYPE (argarray[0])),
-                               DECL_CONTEXT (fn),
-                               ba_any, NULL);
+      tree binfo = lookup_base_sfinae (TREE_TYPE (TREE_TYPE (argarray[0])),
+                                      DECL_CONTEXT (fn),
+                                      ba_any, NULL, complain);
       gcc_assert (binfo && binfo != error_mark_node);
 
       /* Warn about deprecated virtual functions now, since we're about
Index: cvt.c
===================================================================
--- cvt.c       (revision 190207)
+++ cvt.c       (working copy)
@@ -149,11 +149,13 @@ cp_convert_to_pointer (tree type, tree expr, tsubs
          binfo = NULL_TREE;
          /* Try derived to base conversion.  */
          if (!same_p)
-           binfo = lookup_base (intype_class, type_class, ba_check, NULL);
+           binfo = lookup_base_sfinae (intype_class, type_class, ba_check,
+                                       NULL, complain);
          if (!same_p && !binfo)
            {
              /* Try base to derived conversion.  */
-             binfo = lookup_base (type_class, intype_class, ba_check, NULL);
+             binfo = lookup_base_sfinae (type_class, intype_class, ba_check,
+                                         NULL, complain);
              code = MINUS_EXPR;
            }
          if (binfo == error_mark_node)
@@ -278,12 +280,12 @@ convert_to_pointer_force (tree type, tree expr, ts
          enum tree_code code = PLUS_EXPR;
          tree binfo;
 
-         binfo = lookup_base (TREE_TYPE (intype), TREE_TYPE (type),
-                              ba_unique, NULL);
+         binfo = lookup_base_sfinae (TREE_TYPE (intype), TREE_TYPE (type),
+                                     ba_unique, NULL, complain);
          if (!binfo)
            {
-             binfo = lookup_base (TREE_TYPE (type), TREE_TYPE (intype),
-                                  ba_unique, NULL);
+             binfo = lookup_base_sfinae (TREE_TYPE (type), TREE_TYPE (intype),
+                                         ba_unique, NULL, complain);
              code = MINUS_EXPR;
            }
          if (binfo == error_mark_node)
@@ -351,8 +353,9 @@ build_up_reference (tree type, tree arg, int flags
       && MAYBE_CLASS_TYPE_P (argtype)
       && MAYBE_CLASS_TYPE_P (target_type))
     {
-      /* We go through lookup_base for the access control.  */
-      tree binfo = lookup_base (argtype, target_type, ba_check, NULL);
+      /* We go through lookup_base_sfinae for the access control.  */
+      tree binfo = lookup_base_sfinae (argtype, target_type, ba_check,
+                                      NULL, complain);
       if (binfo == error_mark_node)
        return error_mark_node;
       if (binfo == NULL_TREE)
Index: cp-tree.h
===================================================================
--- cp-tree.h   (revision 190207)
+++ cp-tree.h   (working copy)
@@ -5439,6 +5439,8 @@ extern bool emit_tinfo_decl                       (tree);
 
 /* in search.c */
 extern bool accessible_base_p                  (tree, tree, bool);
+extern tree lookup_base_sfinae                 (tree, tree, base_access,
+                                                base_kind *, tsubst_flags_t);
 extern tree lookup_base                                (tree, tree, 
base_access,
                                                 base_kind *);
 extern tree dcast_base_hint                    (tree, tree);
Index: search.c
===================================================================
--- search.c    (revision 190207)
+++ search.c    (working copy)
@@ -185,7 +185,8 @@ accessible_base_p (tree t, tree base, bool conside
    NULL_TREE is returned.  */
 
 tree
-lookup_base (tree t, tree base, base_access access, base_kind *kind_ptr)
+lookup_base_sfinae (tree t, tree base, base_access access,
+                   base_kind *kind_ptr, tsubst_flags_t complain)
 {
   tree binfo;
   tree t_binfo;
@@ -253,7 +254,8 @@ tree
       case bk_ambig:
        if (!(access & ba_quiet))
          {
-           error ("%qT is an ambiguous base of %qT", base, t);
+           if (complain & tf_error)
+             error ("%qT is an ambiguous base of %qT", base, t);
            binfo = error_mark_node;
          }
        break;
@@ -271,7 +273,8 @@ tree
          {
            if (!(access & ba_quiet))
              {
-               error ("%qT is an inaccessible base of %qT", base, t);
+               if (complain & tf_error)
+                 error ("%qT is an inaccessible base of %qT", base, t);
                binfo = error_mark_node;
              }
            else
@@ -287,6 +290,12 @@ tree
   return binfo;
 }
 
+tree
+lookup_base (tree t, tree base, base_access access, base_kind *kind_ptr)
+{
+  return lookup_base_sfinae (t, base, access, kind_ptr, tf_warning_or_error);
+}
+
 /* Data for dcast_base_hint walker.  */
 
 struct dcast_data_s
struct B
{};

struct D
  : private B
{};

template<typename T>
T &&declval();

/*
template<typename From, typename = decltype(B{declval<From>()})>
constexpr bool test_braced_cast_to_base(int)
{ return true; }

template<typename>
constexpr bool test_braced_cast_to_base(bool)
{ return false; }

static_assert(!test_braced_cast_to_base<D>(0), "");


template<typename From, typename = decltype(D{declval<From>()})>
constexpr bool test_braced_cast_to_derived(int)
{ return true; }

template<typename>
constexpr bool test_braced_cast_to_derived(bool)
{ return false; }

static_assert(!test_braced_cast_to_derived<B>(0), "");


typedef B *PB;

template<typename From, typename = decltype(PB{declval<From>()})>
constexpr bool test_braced_cast_to_ptr_to_base(int)
{ return true; }

template<typename>
constexpr bool test_braced_cast_to_ptr_to_base(bool)
{ return false; }

static_assert(!test_braced_cast_to_ptr_to_base<D *>(0), "");


typedef D *PD;

template<typename From, typename = decltype(PD{declval<From>()})>
constexpr bool test_braced_cast_to_ptr_to_derived(int)
{ return true; }

template<typename>
constexpr bool test_braced_cast_to_ptr_to_derived(bool)
{ return false; }

static_assert(!test_braced_cast_to_ptr_to_derived<B *>(0), "");
*/

template<typename From, typename To,
         typename = decltype(static_cast<To>(declval<From>()))>
constexpr bool test_static_cast(int)
{ return true; }

template<typename, typename>
constexpr bool test_static_cast(bool)
{ return false; }

static_assert(!test_static_cast<B &, D &>(0), "");
static_assert(!test_static_cast<B *, D *>(0), "");


template<typename From, typename To,
         typename = decltype(dynamic_cast<To>(declval<From>()))>
constexpr bool test_dynamic_cast(int)
{ return true; }

template<typename, typename>
constexpr bool test_dynamic_cast(bool)
{ return false; }

static_assert(!test_dynamic_cast<D &, B &>(0), "");
static_assert(!test_dynamic_cast<D *, B *>(0), "");


int B::*pm = 0;

template<typename T, typename = decltype(declval<T>().*pm)>
constexpr bool test_member_ptr_dot(int)
{ return true; }

template<typename>
constexpr bool test_member_ptr_dot(bool)
{ return false; }

static_assert(!test_member_ptr_dot<D>(0), "");


template<typename T, typename = decltype(declval<T>()->*pm)>
constexpr bool test_member_ptr_arrow(int)
{ return true; }

template<typename>
constexpr bool test_member_ptr_arrow(bool)
{ return false; }

static_assert(!test_member_ptr_arrow<D *>(0), "");


template<typename T, typename U,
         typename = decltype(declval<T>() < declval<U>())>
constexpr bool test_rel_op(int)
{ return true; }

template<typename, typename>
constexpr bool test_rel_op(bool)
{ return false; }

static_assert(!test_rel_op<D *, B *>(0), "");


template<typename T, typename U,
         typename = decltype(declval<T>() == declval<U>())>
constexpr bool test_eq(int)
{ return true; }

template<typename, typename>
constexpr bool test_eq(bool)
{ return false; }

static_assert(!test_eq<D *, B *>(0), "");

/*
template<typename T, typename U,
         typename = decltype(false ? declval<T>() : declval<U>())>
constexpr bool test_cond_op(int)
{ return true; }

template<typename, typename>
constexpr bool test_cond_op(bool)
{ return false; }

static_assert(!test_cond_op<B *, D *>(0), "");
*/

Reply via email to