Message ID | 20200501152314.3886332-1-ppalka@redhat.com |
---|---|
State | New |
Headers | show |
Series | c++: Missing SFINAE with inaccessible static data member [PR90880] | expand |
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"); > +} >
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"); +}