c++: Missing SFINAE with inaccessible static data member [PR90880]

This is a missing SFINAE issue when verifying the accessibility of a
static data member.

The reason is that check_accessibility_of_qualified_id unconditionally
passes tf_warning_or_error to perform_or_defer_access_check, even when
called from tsubst_qualified_id(..., complain=tf_none).

This patch fixes this by plumbing 'complain' from tsubst_qualified_id
through check_accessibility_of_qualified_id to reach
perform_or_defer_access_check, and by giving
check_accessibility_of_qualified_id the appropriate return value.

Bootstrapped and regtested on x64_64-pc-linux-gnu, does this look OK to
commit to trunk?

gcc/cp/ChangeLog:

	PR c++/90880
	* cp-tree.h (check_accessibility_of_qualified_id): Add
	tsubst_flags_t parameter and change return type to bool.
	* parser.c (cp_parser_lookup_name): Pass tf_warning_to_error to
	check_accessibility_of_qualified_id.
	* pt.c (tsubst_qualified_id): Return error_mark_node if
	check_accessibility_of_qualified_id returns false.
	* semantics.c (check_accessibility_of_qualified_id): Add
	complain parameter.  Pass complain instead of
	tf_warning_or_error to perform_or_defer_access_check.  Return
	true unless perform_or_defer_access_check returns false.

gcc/testsuite/ChangeLog:

	PR c++/90880
	* g++.dg/template/sfinae29.C: New test.
---
 gcc/cp/cp-tree.h                         |  2 +-
 gcc/cp/parser.c                          |  3 ++-
 gcc/cp/pt.c                              |  5 +++--
 gcc/cp/semantics.c                       | 18 ++++++++++-------
 gcc/testsuite/g++.dg/template/sfinae29.C | 25 ++++++++++++++++++++++++
 5 files changed, 42 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/sfinae29.C

On 5/1/20 11:23 AM, Patrick Palka wrote:
> This is a missing SFINAE issue when verifying the accessibility of a
> static data member.
> 
> The reason is that check_accessibility_of_qualified_id unconditionally
> passes tf_warning_or_error to perform_or_defer_access_check, even when
> called from tsubst_qualified_id(..., complain=tf_none).
> 
> This patch fixes this by plumbing 'complain' from tsubst_qualified_id
> through check_accessibility_of_qualified_id to reach
> perform_or_defer_access_check, and by giving
> check_accessibility_of_qualified_id the appropriate return value.
> 
> Bootstrapped and regtested on x64_64-pc-linux-gnu, does this look OK to
> commit to trunk?

OK.

> gcc/cp/ChangeLog:
> 
> 	PR c++/90880
> 	* cp-tree.h (check_accessibility_of_qualified_id): Add
> 	tsubst_flags_t parameter and change return type to bool.
> 	* parser.c (cp_parser_lookup_name): Pass tf_warning_to_error to
> 	check_accessibility_of_qualified_id.
> 	* pt.c (tsubst_qualified_id): Return error_mark_node if
> 	check_accessibility_of_qualified_id returns false.
> 	* semantics.c (check_accessibility_of_qualified_id): Add
> 	complain parameter.  Pass complain instead of
> 	tf_warning_or_error to perform_or_defer_access_check.  Return
> 	true unless perform_or_defer_access_check returns false.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/90880
> 	* g++.dg/template/sfinae29.C: New test.
> ---
>   gcc/cp/cp-tree.h                         |  2 +-
>   gcc/cp/parser.c                          |  3 ++-
>   gcc/cp/pt.c                              |  5 +++--
>   gcc/cp/semantics.c                       | 18 ++++++++++-------
>   gcc/testsuite/g++.dg/template/sfinae29.C | 25 ++++++++++++++++++++++++
>   5 files changed, 42 insertions(+), 11 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/sfinae29.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index e115a8a1d25..c4b81428e14 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7258,7 +7258,7 @@ extern bool expand_or_defer_fn_1		(tree);
>   extern void expand_or_defer_fn			(tree);
>   extern void add_typedef_to_current_template_for_access_check (tree, tree,
>   							      location_t);
> -extern void check_accessibility_of_qualified_id (tree, tree, tree);
> +extern bool check_accessibility_of_qualified_id (tree, tree, tree, tsubst_flags_t);
>   extern tree finish_qualified_id_expr		(tree, tree, bool, bool,
>   						 bool, bool, tsubst_flags_t);
>   extern void simplify_aggr_init_expr		(tree *);
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index e1f9786893a..337f22d2784 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -28394,7 +28394,8 @@ cp_parser_lookup_name (cp_parser *parser, tree name,
>        During an explicit instantiation, access is not checked at all,
>        as per [temp.explicit].  */
>     if (DECL_P (decl))
> -    check_accessibility_of_qualified_id (decl, object_type, parser->scope);
> +    check_accessibility_of_qualified_id (decl, object_type, parser->scope,
> +					 tf_warning_or_error);
>   
>     maybe_record_typedef_use (decl);
>   
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index d28585efd17..e48d3f791ac 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -16171,8 +16171,9 @@ tsubst_qualified_id (tree qualified_id, tree args,
>   
>     if (DECL_P (expr))
>       {
> -      check_accessibility_of_qualified_id (expr, /*object_type=*/NULL_TREE,
> -					   scope);
> +      if (!check_accessibility_of_qualified_id (expr, /*object_type=*/NULL_TREE,
> +						scope, complain))
> +	return error_mark_node;
>         /* Remember that there was a reference to this entity.  */
>         if (!mark_used (expr, complain) && !(complain & tf_error))
>   	return error_mark_node;
> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> index 64a7b76d437..4d1592ab0d2 100644
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -2028,12 +2028,14 @@ add_typedef_to_current_template_for_access_check (tree typedef_decl,
>      an error message if it is not accessible.  If OBJECT_TYPE is
>      non-NULL, we have just seen `x->' or `x.' and OBJECT_TYPE is the
>      type of `*x', or `x', respectively.  If the DECL was named as
> -   `A::B' then NESTED_NAME_SPECIFIER is `A'.  */
> +   `A::B' then NESTED_NAME_SPECIFIER is `A'.  Return value is like
> +   perform_access_checks above.  */
>   
> -void
> +bool
>   check_accessibility_of_qualified_id (tree decl,
>   				     tree object_type,
> -				     tree nested_name_specifier)
> +				     tree nested_name_specifier,
> +				     tsubst_flags_t complain)
>   {
>     tree scope;
>     tree qualifying_type = NULL_TREE;
> @@ -2050,13 +2052,13 @@ check_accessibility_of_qualified_id (tree decl,
>   
>     /* If we're not checking, return immediately.  */
>     if (deferred_access_no_check)
> -    return;
> +    return true;
>   
>     /* Determine the SCOPE of DECL.  */
>     scope = context_for_name_lookup (decl);
>     /* If the SCOPE is not a type, then DECL is not a member.  */
>     if (!TYPE_P (scope))
> -    return;
> +    return true;
>     /* Compute the scope through which DECL is being accessed.  */
>     if (object_type
>         /* OBJECT_TYPE might not be a class type; consider:
> @@ -2097,8 +2099,10 @@ check_accessibility_of_qualified_id (tree decl,
>   	 or similar in a default argument value.  */
>         && CLASS_TYPE_P (qualifying_type)
>         && !dependent_type_p (qualifying_type))
> -    perform_or_defer_access_check (TYPE_BINFO (qualifying_type), decl,
> -				   decl, tf_warning_or_error);
> +    return perform_or_defer_access_check (TYPE_BINFO (qualifying_type), decl,
> +					  decl, complain);
> +
> +  return true;
>   }
>   
>   /* EXPR is the result of a qualified-id.  The QUALIFYING_CLASS was the
> diff --git a/gcc/testsuite/g++.dg/template/sfinae29.C b/gcc/testsuite/g++.dg/template/sfinae29.C
> new file mode 100644
> index 00000000000..0ccaea8593d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/sfinae29.C
> @@ -0,0 +1,25 @@
> +// PR c++/90880
> +// { dg-do compile { target c++11 } }
> +
> +template <typename T, typename = void>
> +struct status
> +{ static const bool value = false; };
> +
> +template <typename T>
> +struct status<T, decltype((void)T::member)>
> +{ static const bool value = true; };
> +
> +struct s1{int member;};
> +struct s2{int _member;};
> +
> +class c1{int member;};
> +class c2{int _member;};
> +
> +void
> +foo()
> +{
> +  static_assert(status<s1>::value, "has member");
> +  static_assert(!status<s2>::value, "has no member");
> +  static_assert(!status<c1>::value, "has inaccessible member");
> +  static_assert(!status<c2>::value, "has no member");
> +}
>